On Thu, Oct 04, 2018 at 11:25:33AM +0200, Daniel Borkmann wrote:
> On 10/04/2018 02:03 AM, Pablo Neira Ayuso wrote:
> > This new field allows you to restrict the metadata template for a given
> > tunnel driver. This is convenient in scenarios that combine different
> > tunneling drivers, to deal with possible misconfigurations given that
> > the template can be interpreted by any target tunnel driver. Default
> > value is IP_TUNNEL_TYPE_UNSPEC, to retain the existing behaviour. This
> > also implicitly exposes what drivers are currently supported in the
> > IP_TUNNEL_INFO_TX mode.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> > ---
> > drivers/net/geneve.c | 3 ++-
> > drivers/net/vxlan.c | 13 +++++++------
> > include/net/dst_metadata.h | 1 +
> > include/net/ip_tunnels.h | 16 ++++++++++++++++
> > net/ipv4/ip_gre.c | 2 ++
> > net/ipv6/ip6_gre.c | 2 ++
> > net/openvswitch/flow_netlink.c | 1 +
> > 7 files changed, 31 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> > index 6625fabe2c88..c383c394f0d2 100644
> > --- a/drivers/net/geneve.c
> > +++ b/drivers/net/geneve.c
> > @@ -920,7 +920,8 @@ static netdev_tx_t geneve_xmit(struct sk_buff *skb, struct net_device *dev)
> >
> > if (geneve->collect_md) {
> > info = skb_tunnel_info(skb);
> > - if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
> > + if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX) ||
> > + !ip_tunnel_type(info, IP_TUNNEL_TYPE_GENEVE))) {
> > err = -EINVAL;
> > netdev_dbg(dev, "no tunnel metadata\n");
> > goto tx_error;
> > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> > index e5d236595206..8cca91b572bd 100644
> > --- a/drivers/net/vxlan.c
> > +++ b/drivers/net/vxlan.c
> > @@ -2296,14 +2296,15 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
> > skb_reset_mac_header(skb);
> >
> > if (vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA) {
> > - if (info && info->mode & IP_TUNNEL_INFO_BRIDGE &&
> > - info->mode & IP_TUNNEL_INFO_TX) {
> > + if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX) ||
> > + !ip_tunnel_type(info, IP_TUNNEL_TYPE_VXLAN))) {
> > + kfree_skb(skb);
> > + return NETDEV_TX_OK;
> > + }
> > + if (info->mode & IP_TUNNEL_INFO_BRIDGE) {
> > vni = tunnel_id_to_key32(info->key.tun_id);
> > } else {
> > - if (info && info->mode & IP_TUNNEL_INFO_TX)
> > - vxlan_xmit_one(skb, dev, vni, NULL, false);
> > - else
> > - kfree_skb(skb);
> > + vxlan_xmit_one(skb, dev, vni, NULL, false);
> > return NETDEV_TX_OK;
> > }
> > }
> > diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
> > index 56cb3c38569a..674116f7fa4a 100644
> > --- a/include/net/dst_metadata.h
> > +++ b/include/net/dst_metadata.h
> > @@ -100,6 +100,7 @@ static inline struct metadata_dst *tun_rx_dst(int md_size)
> > if (!tun_dst)
> > return NULL;
> >
> > + tun_dst->u.tun_info.type = IP_TUNNEL_TYPE_UNSPEC;
> > tun_dst->u.tun_info.options_len = 0;
> > tun_dst->u.tun_info.mode = 0;
> > return tun_dst;
> > diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> > index b0d022ff6ea1..985d24b6a102 100644
> > --- a/include/net/ip_tunnels.h
> > +++ b/include/net/ip_tunnels.h
> > @@ -66,7 +66,16 @@ struct ip_tunnel_key {
> > GENMASK((FIELD_SIZEOF(struct ip_tunnel_info, \
> > options_len) * BITS_PER_BYTE) - 1, 0)
> >
> > +enum ip_tunnel_type {
> > + IP_TUNNEL_TYPE_UNSPEC = 0,
> > + IP_TUNNEL_TYPE_GRE,
> > + IP_TUNNEL_TYPE_VXLAN,
> > + IP_TUNNEL_TYPE_GENEVE,
> > + IP_TUNNEL_TYPE_ERSPAN,
> > +};
> > +
> > struct ip_tunnel_info {
> > + enum ip_tunnel_type type;
> > struct ip_tunnel_key key;
> > #ifdef CONFIG_DST_CACHE
> > struct dst_cache dst_cache;
> > @@ -75,6 +84,13 @@ struct ip_tunnel_info {
> > u8 mode;
> > };
> >
> > +static inline bool ip_tunnel_type(const struct ip_tunnel_info *tun_info,
> > + enum ip_tunnel_type type)
> > +{
> > + return tun_info->type == IP_TUNNEL_TYPE_UNSPEC ||
> > + tun_info->type == type;
> > +}
> > +
> > /* 6rd prefix/relay information */
> > #ifdef CONFIG_IPV6_SIT_6RD
> > struct ip_tunnel_6rd_parm {
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index c3385a84f8ff..3ad12135c3d3 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -534,6 +534,7 @@ static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev,
> >
> > tun_info = skb_tunnel_info(skb);
> > if (unlikely(!tun_info || !(tun_info->mode & IP_TUNNEL_INFO_TX) ||
> > + !ip_tunnel_type(tun_info, IP_TUNNEL_TYPE_GRE) ||
> > ip_tunnel_info_af(tun_info) != AF_INET))
> > goto err_free_skb;
> >
> > @@ -585,6 +586,7 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
> >
> > tun_info = skb_tunnel_info(skb);
> > if (unlikely(!tun_info || !(tun_info->mode & IP_TUNNEL_INFO_TX) ||
> > + !ip_tunnel_type(tun_info, IP_TUNNEL_TYPE_ERSPAN) ||
> > ip_tunnel_info_af(tun_info) != AF_INET))
> > goto err_free_skb;
> >
> > diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> > index 515adbdba1d2..675f373809ee 100644
> > --- a/net/ipv6/ip6_gre.c
> > +++ b/net/ipv6/ip6_gre.c
> > @@ -732,6 +732,7 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb,
> > tun_info = skb_tunnel_info(skb);
> > if (unlikely(!tun_info ||
> > !(tun_info->mode & IP_TUNNEL_INFO_TX) ||
> > + !ip_tunnel_type(tun_info, IP_TUNNEL_TYPE_GRE) ||
> > ip_tunnel_info_af(tun_info) != AF_INET6))
> > return -EINVAL;
> >
> > @@ -960,6 +961,7 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
> > tun_info = skb_tunnel_info(skb);
> > if (unlikely(!tun_info ||
> > !(tun_info->mode & IP_TUNNEL_INFO_TX) ||
> > + !ip_tunnel_type(tun_info, IP_TUNNEL_TYPE_ERSPAN) ||
> > ip_tunnel_info_af(tun_info) != AF_INET6))
> > return -EINVAL;
> >
> > diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> > index a70097ecf33c..1ee2509534df 100644
> > --- a/net/openvswitch/flow_netlink.c
> > +++ b/net/openvswitch/flow_netlink.c
> > @@ -2602,6 +2602,7 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
> > ovs_tun->tun_dst = tun_dst;
> >
> > tun_info = &tun_dst->u.tun_info;
> > + tun_info->type = IP_TUNNEL_TYPE_UNSPEC;
> > tun_info->mode = IP_TUNNEL_INFO_TX;
> > if (key.tun_proto == AF_INET6)
> > tun_info->mode |= IP_TUNNEL_INFO_IPV6;
> >
>
> If so then this should also be made explicit IP_TUNNEL_TYPE_UNSPEC in BPF code
> since all these tunnel types are supported there as well.
Are you refering to proper initialization? I can see a memset() there
for the ip_tunnel_info structure, which is implicitly setting
tun_info->type to zero, ie. IP_TUNNEL_TYPE_UNSPEC.
I can also make it explicit there if you prefer.
Thanks.
