| Message ID | 1da8fb9d3af8dcee1948903ae816438578365e51.1570455278.git.martinvarghesenokia@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Bareudp Tunnel Module | expand |
On 10/8/19 2:49 AM, Martin Varghese wrote: > From: Martin <martin.varghese@nokia.com> > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> drop one of those. > --- > Documentation/networking/bareudp.txt | 18 ++++++++ > drivers/net/bareudp.c | 82 +++++++++++++++++++++++++++++++++--- > include/net/bareudp.h | 1 + > include/uapi/linux/if_link.h | 1 + > 4 files changed, 95 insertions(+), 7 deletions(-) > > diff --git a/Documentation/networking/bareudp.txt b/Documentation/networking/bareudp.txt > index d2530e2..4de1022 100644 > --- a/Documentation/networking/bareudp.txt > +++ b/Documentation/networking/bareudp.txt > @@ -9,6 +9,15 @@ The Bareudp tunnel module provides a generic L3 encapsulation tunnelling > support for tunnelling different L3 protocols like MPLS, IP, NSH etc. inside > a UDP tunnel. > > +Special Handling > +---------------- > +The bareudp device supports special handling for MPLS & IP as they can have > +multiple ethertypes. > +MPLS procotcol can have ethertypes 0x8847 (unicast) & 0x8847 (multicast). 0x8848 > +IP proctocol can have ethertypes 0x0800 (v4) & 0x866 (v6). > +This special handling can be enabled only for ethertype 0x0800 & 0x88847 with a > +flag called extended mode. > + > Usage > ------ > > @@ -21,3 +30,12 @@ This creates a bareudp tunnel device which tunnels L3 traffic with ethertype > The device will listen on UDP port 6635 to receive traffic. > > b. ip link delete bareudp0 > + > +2. Device creation with extended mode enabled > + > +There are two ways to create a bareudp device for MPLS & IP with extended mode > +enabled end that sentence with a period. (or full stop) > + > +a. ip link add dev bareudp0 type bareudp dstport 6635 ethertype 0x8847 extmode 1 > + > +b. ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls
On Tue, Oct 8, 2019 at 5:52 AM Martin Varghese <martinvarghesenokia@gmail.com> wrote: > > From: Martin <martin.varghese@nokia.com> > This commit would need a commit message. > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > --- > Documentation/networking/bareudp.txt | 18 ++++++++ > drivers/net/bareudp.c | 82 +++++++++++++++++++++++++++++++++--- > include/net/bareudp.h | 1 + > include/uapi/linux/if_link.h | 1 + > 4 files changed, 95 insertions(+), 7 deletions(-) > > diff --git a/Documentation/networking/bareudp.txt b/Documentation/networking/bareudp.txt > index d2530e2..4de1022 100644 > --- a/Documentation/networking/bareudp.txt > +++ b/Documentation/networking/bareudp.txt > @@ -9,6 +9,15 @@ The Bareudp tunnel module provides a generic L3 encapsulation tunnelling > support for tunnelling different L3 protocols like MPLS, IP, NSH etc. inside > a UDP tunnel. > > +Special Handling > +---------------- > +The bareudp device supports special handling for MPLS & IP as they can have > +multiple ethertypes. Special in what way? > +MPLS procotcol can have ethertypes 0x8847 (unicast) & 0x8847 (multicast). 0x8848. Use ETH_P_MPLS_UC and ETH_P_MPLS_MC instead of hard coding constants. > +IP proctocol can have ethertypes 0x0800 (v4) & 0x866 (v6). > +This special handling can be enabled only for ethertype 0x0800 & 0x88847 with a Again typo. > +flag called extended mode. > + > Usage > ------ > > @@ -21,3 +30,12 @@ This creates a bareudp tunnel device which tunnels L3 traffic with ethertype > The device will listen on UDP port 6635 to receive traffic. > > b. ip link delete bareudp0 > + > +2. Device creation with extended mode enabled > + > +There are two ways to create a bareudp device for MPLS & IP with extended mode > +enabled > + > +a. ip link add dev bareudp0 type bareudp dstport 6635 ethertype 0x8847 extmode 1 > + > +b. ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c > index 7e6813a..2a688da 100644 > --- a/drivers/net/bareudp.c > +++ b/drivers/net/bareudp.c > @@ -48,6 +48,7 @@ struct bareudp_dev { > struct net_device *dev; /* netdev for bareudp tunnel */ > __be16 ethertype; > u16 sport_min; > + bool ext_mode; > struct bareudp_conf conf; > struct bareudp_sock __rcu *sock4; /* IPv4 socket for bareudp tunnel */ > #if IS_ENABLED(CONFIG_IPV6) > @@ -82,15 +83,64 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) > goto drop; > > bareudp = bs->bareudp; > - proto = bareudp->ethertype; > + if (!bareudp) > + goto drop; > + > + if (bareudp->ethertype == htons(ETH_P_IP)) { > + struct iphdr *iphdr; > + > + iphdr = (struct iphdr *)(skb->data + BAREUDP_BASE_HLEN); > + if (iphdr->version == 4) { > + proto = bareudp->ethertype; > + } else if (bareudp->ext_mode && (iphdr->version == 6)) { > + proto = htons(ETH_P_IPV6); Verified packet length before reading at offset? Why does v6 needs extended mode, while v4 does not? For any packet encapsulated in UDP, the inner packet type will be unknown, so needs to be configured on the device. That is not a special feature. FOU gives an example. My main concern is that this introduces a lot of code that nearly duplicates existing tunneling support. It is not clear to me that existing logic cannot be reused/extended.
On Tue, Oct 08, 2019 at 12:09:49PM -0400, Willem de Bruijn wrote: > On Tue, Oct 8, 2019 at 5:52 AM Martin Varghese > <martinvarghesenokia@gmail.com> wrote: > > > > From: Martin <martin.varghese@nokia.com> > > > > This commit would need a commit message. > > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > > > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > > --- > > Documentation/networking/bareudp.txt | 18 ++++++++ > > drivers/net/bareudp.c | 82 +++++++++++++++++++++++++++++++++--- > > include/net/bareudp.h | 1 + > > include/uapi/linux/if_link.h | 1 + > > 4 files changed, 95 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/networking/bareudp.txt b/Documentation/networking/bareudp.txt > > index d2530e2..4de1022 100644 > > --- a/Documentation/networking/bareudp.txt > > +++ b/Documentation/networking/bareudp.txt > > @@ -9,6 +9,15 @@ The Bareudp tunnel module provides a generic L3 encapsulation tunnelling > > support for tunnelling different L3 protocols like MPLS, IP, NSH etc. inside > > a UDP tunnel. > > > > +Special Handling > > +---------------- > > +The bareudp device supports special handling for MPLS & IP as they can have > > +multiple ethertypes. > > Special in what way? > The bareudp device associates a L3 protocol (ethertype) with a UDP port. For some protocols like MPLS,IP there exists multiplle ethertypes. IPV6 and IPV4 ethertypes for IP and MPLS unicast & Multicast ethertypes for MPLS. There coud be use cases where both MPLS unicast and multicast traffic need to be tunnelled using the same bareudp device.Similarly for ipv4 and ipv6. This problem is solved by introducing a flag called extended mode which should be used be with IPv4 and MPLS unicast ethertypes. The extended mode flag when used with IPV4 ethertype enables the bareudp device to recognize & support IPV4 & v6. The extended mode flag when used with MPLS unicast ethertype enables bareudp device to recognize & support MPLS unicast & multicast. > > +MPLS procotcol can have ethertypes 0x8847 (unicast) & 0x8847 (multicast). > > 0x8848. Use ETH_P_MPLS_UC and ETH_P_MPLS_MC instead of hard coding constants. > > > +IP proctocol can have ethertypes 0x0800 (v4) & 0x866 (v6). > > +This special handling can be enabled only for ethertype 0x0800 & 0x88847 with a > > Again typo. > > > +flag called extended mode. > > + > > Usage > > ------ > > > > @@ -21,3 +30,12 @@ This creates a bareudp tunnel device which tunnels L3 traffic with ethertype > > The device will listen on UDP port 6635 to receive traffic. > > > > b. ip link delete bareudp0 > > + > > +2. Device creation with extended mode enabled > > + > > +There are two ways to create a bareudp device for MPLS & IP with extended mode > > +enabled > > + > > +a. ip link add dev bareudp0 type bareudp dstport 6635 ethertype 0x8847 extmode 1 > > + > > +b. ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls > > diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c > > index 7e6813a..2a688da 100644 > > --- a/drivers/net/bareudp.c > > +++ b/drivers/net/bareudp.c > > @@ -48,6 +48,7 @@ struct bareudp_dev { > > struct net_device *dev; /* netdev for bareudp tunnel */ > > __be16 ethertype; > > u16 sport_min; > > + bool ext_mode; > > struct bareudp_conf conf; > > struct bareudp_sock __rcu *sock4; /* IPv4 socket for bareudp tunnel */ > > #if IS_ENABLED(CONFIG_IPV6) > > @@ -82,15 +83,64 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) > > goto drop; > > > > bareudp = bs->bareudp; > > - proto = bareudp->ethertype; > > + if (!bareudp) > > + goto drop; > > + > > + if (bareudp->ethertype == htons(ETH_P_IP)) { > > + struct iphdr *iphdr; > > + > > + iphdr = (struct iphdr *)(skb->data + BAREUDP_BASE_HLEN); > > + if (iphdr->version == 4) { > > + proto = bareudp->ethertype; > > + } else if (bareudp->ext_mode && (iphdr->version == 6)) { > > + proto = htons(ETH_P_IPV6); > > Verified packet length before reading at offset? Why does v6 needs > extended mode, while v4 does not? > Explained above. > For any packet encapsulated in UDP, the inner packet type will be > unknown, so needs to be configured on the device. That is not a > special feature. FOU gives an example. My main concern is that this > introduces a lot of code that nearly duplicates existing tunneling > support. It is not clear to me that existing logic cannot be > reused/extended.
On Wed, Oct 9, 2019 at 9:39 AM Martin Varghese <martinvarghesenokia@gmail.com> wrote: > > On Tue, Oct 08, 2019 at 12:09:49PM -0400, Willem de Bruijn wrote: > > On Tue, Oct 8, 2019 at 5:52 AM Martin Varghese > > <martinvarghesenokia@gmail.com> wrote: > > > > > > From: Martin <martin.varghese@nokia.com> > > > > > > > This commit would need a commit message. > > > > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > > > > > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > > > --- > > > Documentation/networking/bareudp.txt | 18 ++++++++ > > > drivers/net/bareudp.c | 82 +++++++++++++++++++++++++++++++++--- > > > include/net/bareudp.h | 1 + > > > include/uapi/linux/if_link.h | 1 + > > > 4 files changed, 95 insertions(+), 7 deletions(-) > > > > > > diff --git a/Documentation/networking/bareudp.txt b/Documentation/networking/bareudp.txt > > > index d2530e2..4de1022 100644 > > > --- a/Documentation/networking/bareudp.txt > > > +++ b/Documentation/networking/bareudp.txt > > > @@ -9,6 +9,15 @@ The Bareudp tunnel module provides a generic L3 encapsulation tunnelling > > > support for tunnelling different L3 protocols like MPLS, IP, NSH etc. inside > > > a UDP tunnel. > > > > > > +Special Handling > > > +---------------- > > > +The bareudp device supports special handling for MPLS & IP as they can have > > > +multiple ethertypes. > > > > Special in what way? > > > The bareudp device associates a L3 protocol (ethertype) with a UDP port. > For some protocols like MPLS,IP there exists multiplle ethertypes. > IPV6 and IPV4 ethertypes for IP and MPLS unicast & Multicast ethertypes for > MPLS. There coud be use cases where both MPLS unicast and multicast traffic > need to be tunnelled using the same bareudp device.Similarly for ipv4 and ipv6. IP is already solved. I would focus on MPLS. Also, the days where IPv6 is optional (and needs IPv4 enabled) are behind us, really. Maybe just let the admin explicitly specify MPLS unicast, multicast or both, instead of defining a new extended label.
On Wed, Oct 9, 2019 at 11:06 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > On Wed, Oct 9, 2019 at 9:39 AM Martin Varghese > <martinvarghesenokia@gmail.com> wrote: > > > > On Tue, Oct 08, 2019 at 12:09:49PM -0400, Willem de Bruijn wrote: > > > On Tue, Oct 8, 2019 at 5:52 AM Martin Varghese > > > <martinvarghesenokia@gmail.com> wrote: > > > > > > > > From: Martin <martin.varghese@nokia.com> > > > > > > > > > > This commit would need a commit message. > > > > > > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > > > > > > > > Signed-off-by: Martin Varghese <martinvarghesenokia@gmail.com> > > > > --- > > > > Documentation/networking/bareudp.txt | 18 ++++++++ > > > > drivers/net/bareudp.c | 82 +++++++++++++++++++++++++++++++++--- > > > > include/net/bareudp.h | 1 + > > > > include/uapi/linux/if_link.h | 1 + > > > > 4 files changed, 95 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/Documentation/networking/bareudp.txt b/Documentation/networking/bareudp.txt > > > > index d2530e2..4de1022 100644 > > > > --- a/Documentation/networking/bareudp.txt > > > > +++ b/Documentation/networking/bareudp.txt > > > > @@ -9,6 +9,15 @@ The Bareudp tunnel module provides a generic L3 encapsulation tunnelling > > > > support for tunnelling different L3 protocols like MPLS, IP, NSH etc. inside > > > > a UDP tunnel. > > > > > > > > +Special Handling > > > > +---------------- > > > > +The bareudp device supports special handling for MPLS & IP as they can have > > > > +multiple ethertypes. > > > > > > Special in what way? > > > > > The bareudp device associates a L3 protocol (ethertype) with a UDP port. > > For some protocols like MPLS,IP there exists multiplle ethertypes. > > IPV6 and IPV4 ethertypes for IP and MPLS unicast & Multicast ethertypes for > > MPLS. There coud be use cases where both MPLS unicast and multicast traffic > > need to be tunnelled using the same bareudp device.Similarly for ipv4 and ipv6. > > IP is already solved. I would focus on MPLS. > > Also, the days where IPv6 is optional (and needs IPv4 enabled) are > behind us, really. Ah sorry, there is nothing stopping someone from creating an ETH_P_IPV6 only device before this path. > Maybe just let the admin explicitly specify MPLS unicast, multicast or > both, instead of defining a new extended label. Deriving the inner protocol type from the outer protocol mode sounds fine, indeed.
diff --git a/Documentation/networking/bareudp.txt b/Documentation/networking/bareudp.txt index d2530e2..4de1022 100644 --- a/Documentation/networking/bareudp.txt +++ b/Documentation/networking/bareudp.txt @@ -9,6 +9,15 @@ The Bareudp tunnel module provides a generic L3 encapsulation tunnelling support for tunnelling different L3 protocols like MPLS, IP, NSH etc. inside a UDP tunnel. +Special Handling +---------------- +The bareudp device supports special handling for MPLS & IP as they can have +multiple ethertypes. +MPLS procotcol can have ethertypes 0x8847 (unicast) & 0x8847 (multicast). +IP proctocol can have ethertypes 0x0800 (v4) & 0x866 (v6). +This special handling can be enabled only for ethertype 0x0800 & 0x88847 with a +flag called extended mode. + Usage ------ @@ -21,3 +30,12 @@ This creates a bareudp tunnel device which tunnels L3 traffic with ethertype The device will listen on UDP port 6635 to receive traffic. b. ip link delete bareudp0 + +2. Device creation with extended mode enabled + +There are two ways to create a bareudp device for MPLS & IP with extended mode +enabled + +a. ip link add dev bareudp0 type bareudp dstport 6635 ethertype 0x8847 extmode 1 + +b. ip link add dev bareudp0 type bareudp dstport 6635 ethertype mpls diff --git a/drivers/net/bareudp.c b/drivers/net/bareudp.c index 7e6813a..2a688da 100644 --- a/drivers/net/bareudp.c +++ b/drivers/net/bareudp.c @@ -48,6 +48,7 @@ struct bareudp_dev { struct net_device *dev; /* netdev for bareudp tunnel */ __be16 ethertype; u16 sport_min; + bool ext_mode; struct bareudp_conf conf; struct bareudp_sock __rcu *sock4; /* IPv4 socket for bareudp tunnel */ #if IS_ENABLED(CONFIG_IPV6) @@ -82,15 +83,64 @@ static int bareudp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) goto drop; bareudp = bs->bareudp; - proto = bareudp->ethertype; + if (!bareudp) + goto drop; + + if (bareudp->ethertype == htons(ETH_P_IP)) { + struct iphdr *iphdr; + + iphdr = (struct iphdr *)(skb->data + BAREUDP_BASE_HLEN); + if (iphdr->version == 4) { + proto = bareudp->ethertype; + } else if (bareudp->ext_mode && (iphdr->version == 6)) { + proto = htons(ETH_P_IPV6); + } else { + bareudp->dev->stats.rx_dropped++; + goto drop; + } + } else if (bareudp->ethertype == htons(ETH_P_MPLS_UC)) { + struct iphdr *tunnel_hdr; + + tunnel_hdr = (struct iphdr *)skb_network_header(skb); + if (tunnel_hdr->version == 4) { + if (!ipv4_is_multicast(tunnel_hdr->daddr)) { + proto = bareudp->ethertype; + } else if (bareudp->ext_mode && + ipv4_is_multicast(tunnel_hdr->daddr)) { + proto = htons(ETH_P_MPLS_MC); + } else { + bareudp->dev->stats.rx_dropped++; + goto drop; + } + } else { + int addr_type; + struct ipv6hdr *tunnel_hdr_v6; + + tunnel_hdr_v6 = (struct ipv6hdr *)skb_network_header(skb); + addr_type = + ipv6_addr_type((struct in6_addr *)&tunnel_hdr_v6->daddr); + if (!(addr_type & IPV6_ADDR_MULTICAST)) { + proto = bareudp->ethertype; + } else if (bareudp->ext_mode && + (addr_type & IPV6_ADDR_MULTICAST)) { + proto = htons(ETH_P_MPLS_MC); + } else { + bareudp->dev->stats.rx_dropped++; + goto drop; + } + } + } else { + proto = bareudp->ethertype; + } if (iptunnel_pull_header(skb, BAREUDP_BASE_HLEN, - proto, - !net_eq(bareudp->net, - dev_net(bareudp->dev)))) { + proto, + !net_eq(bareudp->net, + dev_net(bareudp->dev)))) { bareudp->dev->stats.rx_dropped++; goto drop; } + tun_dst = udp_tun_rx_dst(skb, bareudp_get_sk_family(bs), TUNNEL_KEY, 0, 0); if (!tun_dst) { @@ -522,10 +572,13 @@ static netdev_tx_t bareudp_xmit(struct sk_buff *skb, struct net_device *dev) int err; if (skb->protocol != bareudp->ethertype) { - err = -EINVAL; - goto tx_error; + if (!bareudp->ext_mode || + (skb->protocol != htons(ETH_P_MPLS_MC) && + skb->protocol != htons(ETH_P_IPV6))) { + err = -EINVAL; + goto tx_error; + } } - info = skb_tunnel_info(skb); if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) { err = -EINVAL; @@ -630,6 +683,7 @@ static int bareudp_change_mtu(struct net_device *dev, int new_mtu) [IFLA_BAREUDP_PORT] = { .type = NLA_U16 }, [IFLA_BAREUDP_ETHERTYPE] = { .type = NLA_U16 }, [IFLA_BAREUDP_SRCPORT_MIN] = { .type = NLA_U16 }, + [IFLA_BAREUDP_EXTMODE] = { .type = NLA_FLAG }, }; static int bareudp_validate(struct nlattr *tb[], struct nlattr *data[], @@ -712,9 +766,15 @@ static int bareudp_configure(struct net *net, struct net_device *dev, if (t) return -EBUSY; + if (conf->ext_mode && + (conf->ethertype != htons(ETH_P_MPLS_UC) && + conf->ethertype != htons(ETH_P_IP))) + return -EINVAL; + bareudp->conf = *conf; bareudp->ethertype = conf->ethertype; bareudp->sport_min = conf->sport_min; + bareudp->ext_mode = conf->ext_mode; err = register_netdevice(dev); if (err) return err; @@ -737,6 +797,11 @@ static int bareudp2info(struct nlattr *data[], struct bareudp_conf *conf) if (data[IFLA_BAREUDP_SRCPORT_MIN]) conf->sport_min = nla_get_u16(data[IFLA_BAREUDP_SRCPORT_MIN]); + if (data[IFLA_BAREUDP_EXTMODE]) + conf->ext_mode = true; + else + conf->ext_mode = false; + return 0; } @@ -779,6 +844,7 @@ static size_t bareudp_get_size(const struct net_device *dev) return nla_total_size(sizeof(__be16)) + /* IFLA_BAREUDP_PORT */ nla_total_size(sizeof(__be16)) + /* IFLA_BAREUDP_ETHERTYPE */ nla_total_size(sizeof(__u16)) + /* IFLA_BAREUDP_SRCPORT_MIN */ + nla_total_size(0) + /* IFLA_BAREUDP_EXTMODE */ 0; } @@ -792,6 +858,8 @@ static int bareudp_fill_info(struct sk_buff *skb, const struct net_device *dev) goto nla_put_failure; if (nla_put_u16(skb, IFLA_BAREUDP_SRCPORT_MIN, bareudp->conf.sport_min)) goto nla_put_failure; + if (bareudp->ext_mode && nla_put_flag(skb, IFLA_BAREUDP_EXTMODE)) + goto nla_put_failure; return 0; diff --git a/include/net/bareudp.h b/include/net/bareudp.h index 513fae6..2c121d8 100644 --- a/include/net/bareudp.h +++ b/include/net/bareudp.h @@ -10,6 +10,7 @@ struct bareudp_conf { __be16 ethertype; __be16 port; u16 sport_min; + bool ext_mode; }; struct net_device *bareudp_dev_create(struct net *net, const char *name, diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 012f7e8..2b91c872 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -586,6 +586,7 @@ enum { IFLA_BAREUDP_PORT, IFLA_BAREUDP_ETHERTYPE, IFLA_BAREUDP_SRCPORT_MIN, + IFLA_BAREUDP_EXTMODE, __IFLA_BAREUDP_MAX };