ipmr, ip6mr: Allow MC-routing locally-generated MC packets
Subsystems (10)
v2 cover ipmr, ip6mr: Allow MC-routing locally-generated MC packets
Multicast routing is today handled in the input path. Locally generated MC packets don't hit the IPMR code. Thus if a VXLAN remote address is multicast, the driver needs to set an OIF during route lookup. In practice that means that MC routing configuration needs to be kept in sync with the VXLAN FDB and MDB. Ideally, the VXLAN packets would be routed by the MC routing code instead.
To that end, this patchset adds support to route locally generated multicast packets.
However, an installation that uses a VXLAN underlay netdevice for which it also has matching MC routes, would get a different routing with this patch. Previously, the MC packets would be delivered directly to the underlay port, whereas now they would be MC-routed. In order to avoid this change in behavior, introduce an IPCB/IP6CB flag. Unless the flag is set, the new MC-routing code is skipped.
All this is keyed to a new VXLAN attribute, IFLA_VXLAN_MC_ROUTE. Only when it is set does any of the above engage.
In addition to that, and as is the case today with MC forwarding, IPV4_DEVCONF_MC_FORWARDING must be enabled for the netdevice that acts as a source of MC traffic (i.e. the VXLAN PHYS_DEV), so an MC daemon must be attached to the netdevice.
When a VXLAN netdevice with a MC remote is brought up, the physical netdevice joins the indicated MC group. This is important for local delivery of MC packets, so it is still necessary to configure a physical netdevice -- the parameter cannot go away. The netdevice would however typically not be a front panel port, but a dummy. An MC daemon would then sit on top of that netdevice as well as any front panel ports that it needs to service, and have routes set up between the two.
A way to configure the VXLAN netdevice to take advantage of the new MC routing would be:
# ip link add name d up type dummy # ip link add name vx10 up type vxlan id 1000 dstport 4789 \ local 192.0.2.1 group 225.0.0.1 ttl 16 dev d mrcoute # ip link set dev vx10 master br # plus vlans etc.
With the following MC routes:
(192.0.2.1, 225.0.0.1) iif=d oil=swp1,swp2 # TX route
(*, 225.0.0.1) iif=swp1 oil=d,swp2 # RX route
(*, 225.0.0.1) iif=swp2 oil=d,swp1 # RX route
The RX path has not changed, with the exception of an extra MC hop. Packets are delivered to the front panel port and MC-forwarded to the VXLAN physical port, here "d". Since the port has joined the multicast group, the packets are locally delivered, and end up being processed by the VXLAN netdevice.
This patchset is based on earlier patches from Nikolay Aleksandrov and Roopa Prabhu, though it underwent significant changes. Roopa broadly presented the topic on LPC 2019 [0].
Patchset progression:
- Patches #1 to #4 add ip_mr_output()
- Patches #5 to #9 add ip6_mr_output()
- Patch #10 adds the VXLAN bits to enable MR engagement
- Patches #11 to #13 prepare selftest libraries
- Patch #14 includes a new test suite
[0] https://www.youtube.com/watch?v=xlReECfi-uo
v2:
- Patches #11, #13 and #14:
- Adjust as per shellcheck citations
- Retain Nik's R-b at patch #13, the changes were very minor.
Petr Machata (14):
net: ipv4: Add a flags argument to iptunnel_xmit(),
udp_tunnel_xmit_skb()
net: ipv4: ipmr: ipmr_queue_xmit(): Drop local variable `dev'
net: ipv4: ipmr: Split ipmr_queue_xmit() in two
net: ipv4: Add ip_mr_output()
net: ipv6: Make udp_tunnel6_xmit_skb() void
net: ipv6: Add a flags argument to ip6tunnel_xmit(),
udp_tunnel6_xmit_skb()
net: ipv6: ip6mr: Fix in/out netdev to pass to the FORWARD chain
net: ipv6: ip6mr: Extract a helper out of ip6mr_forward2()
net: ipv6: Add ip6_mr_output()
vxlan: Support MC routing in the underlay
selftests: forwarding: lib: Move smcrouted helpers here
selftests: net: lib: Add ip_link_has_flag()
selftests: forwarding: adf_mcd_start(): Allow configuring custom
interfaces
selftests: forwarding: Add a test for verifying VXLAN MC underlay
drivers/net/amt.c | 9 +- drivers/net/bareudp.c | 7 +- drivers/net/geneve.c | 7 +- drivers/net/gtp.c | 12 +- drivers/net/ovpn/udp.c | 4 +- drivers/net/vxlan/vxlan_core.c | 23 +- drivers/net/wireguard/socket.c | 4 +- include/linux/ipv6.h | 1 + include/linux/mroute6.h | 7 + include/net/ip.h | 2 + include/net/ip6_tunnel.h | 3 +- include/net/ip_tunnels.h | 2 +- include/net/udp_tunnel.h | 17 +- include/net/vxlan.h | 5 +- include/uapi/linux/if_link.h | 1 + net/ipv4/ip_tunnel.c | 4 +- net/ipv4/ip_tunnel_core.c | 4 +- net/ipv4/ipmr.c | 169 +++- net/ipv4/route.c | 2 +- net/ipv4/udp_tunnel_core.c | 5 +- net/ipv6/ip6_tunnel.c | 2 +- net/ipv6/ip6_udp_tunnel.c | 18 +- net/ipv6/ip6mr.c | 137 +++- net/ipv6/route.c | 1 + net/ipv6/sit.c | 2 +- net/sctp/ipv6.c | 7 +- net/sctp/protocol.c | 3 +- net/tipc/udp_media.c | 12 +- .../testing/selftests/net/forwarding/Makefile | 1 + tools/testing/selftests/net/forwarding/lib.sh | 46 ++ .../net/forwarding/router_multicast.sh | 35 +- .../net/forwarding/vxlan_bridge_1q_mc_ul.sh | 769 ++++++++++++++++++ tools/testing/selftests/net/lib.sh | 12 +- 33 files changed, 1216 insertions(+), 117 deletions(-) create mode 100755 tools/testing/selftests/net/forwarding/vxlan_bridge_1q_mc_ul.sh
Signature · 1 line
2.49.0
PATCH v2 1/14 net: ipv4: Add a flags argument to iptunnel_xmit(), udp_tunnel_xmit_skb()
iptunnel_xmit() erases the contents of the SKB control block. In order to be able to set particular IPCB flags on the SKB, add a corresponding parameter, and propagate it to udp_tunnel_xmit_skb() as well.
In one of the following patches, VXLAN driver will use this facility to mark packets as subject to IP multicast routing.
---
Notes:
CC: Pablo Neira Ayuso <[email protected]>
CC: [email protected]
CC: Andrew Lunn <[email protected]>
CC: Taehee Yoo <[email protected]>
CC: Antonio Quartulli <[email protected]>
CC: "Jason A. Donenfeld" <[email protected]>
CC: [email protected]
CC: Marcelo Ricardo Leitner <[email protected]>
CC: [email protected]
CC: Jon Maloy <[email protected]>
CC: [email protected]
drivers/net/amt.c | 9 ++++++--- drivers/net/bareudp.c | 4 ++-- drivers/net/geneve.c | 4 ++-- drivers/net/gtp.c | 10 ++++++---- drivers/net/ovpn/udp.c | 2 +- drivers/net/vxlan/vxlan_core.c | 2 +- drivers/net/wireguard/socket.c | 2 +- include/net/ip_tunnels.h | 2 +- include/net/udp_tunnel.h | 2 +- net/ipv4/ip_tunnel.c | 4 ++-- net/ipv4/ip_tunnel_core.c | 4 +++- net/ipv4/udp_tunnel_core.c | 5 +++-- net/ipv6/sit.c | 2 +- net/sctp/protocol.c | 3 ++- net/tipc/udp_media.c | 2 +- 15 files changed, 33 insertions(+), 24 deletions(-)
drivers/net/amt.c +6 -3
@@ -1046,7 +1046,8 @@ static bool amt_send_membership_update(struct amt_dev *amt, amt->gw_port, amt->relay_port, false, - false); + false, + 0);amt_update_gw_status (amt,AMT_STATUS_SENT_UPDATE , true);return false; }
@@ -1103,7 +1104,8 @@ static void amt_send_multicast_data(struct amt_dev *amt, amt->relay_port, tunnel->source_port, false, - false); + false, + 0); } static bool amt_send_membership_query(struct amt_dev *amt,
@@ -1161,7 +1163,8 @@ static bool amt_send_membership_query(struct amt_dev *amt, amt->relay_port, tunnel->source_port, false, - false); + false, + 0);amt_update_relay_status (tunnel,AMT_STATUS_SENT_QUERY , true);return false; }
drivers/net/bareudp.c +2 -2
@@ -362,8 +362,8 @@ static int bareudp_xmit_skb(struct sk_buff *skb, struct net_device *dev,udp_tunnel_xmit_skb (rt, sock->sk, skb, saddr, info->key.u.ipv4.dst, tos, ttl, df, sport, bareudp->port, !net_eq (bareudp->net,dev_net (bareudp->dev)), - !test_bit (IP_TUNNEL_CSUM_BIT , - info->key.tun_flags)); + !test_bit (IP_TUNNEL_CSUM_BIT , info->key.tun_flags), +0 );return 0 ; free_dst:
drivers/net/geneve.c +2 -2
@@ -921,8 +921,8 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,udp_tunnel_xmit_skb (rt, gs4->sock->sk, skb, saddr, info->key.u.ipv4.dst, tos, ttl, df, sport, geneve->cfg.info.key.tp_dst, !net_eq (geneve->net,dev_net (geneve->dev)), - !test_bit (IP_TUNNEL_CSUM_BIT , - info->key.tun_flags)); + !test_bit (IP_TUNNEL_CSUM_BIT , info->key.tun_flags), +0 );return 0 ; }
drivers/net/gtp.c +6 -4
@@ -446,7 +446,8 @@ static int gtp0_send_echo_resp_ip(struct gtp_dev *gtp, struct sk_buff *skb)htons (GTP0_PORT ),htons (GTP0_PORT ), !net_eq (sock_net (gtp->sk1u),dev_net (gtp->dev)), - false); + false, + 0);return 0 ; }
@@ -704,7 +705,8 @@ static int gtp1u_send_echo_resp(struct gtp_dev *gtp, struct sk_buff *skb)htons (GTP1U_PORT ),htons (GTP1U_PORT ), !net_eq (sock_net (gtp->sk1u),dev_net (gtp->dev)), - false); + false, + 0);return 0 ; }
@@ -1304,7 +1306,7 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev) pktinfo.gtph_port, pktinfo.gtph_port, !net_eq (sock_net (pktinfo.pctx->sk),dev_net (dev)), - false); + false, 0);break ;case AF_INET6 :#if IS_ENABLED (CONFIG_IPV6 )
@@ -2405,7 +2407,7 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info) port, port, !net_eq (sock_net (sk),dev_net (gtp->dev)), - false); + false, 0);return 0 ; }
drivers/net/ovpn/udp.c +1 -1
@@ -199,7 +199,7 @@ static int ovpn_udp4_output(struct ovpn_peer *peer, struct ovpn_bind *bind, transmit:udp_tunnel_xmit_skb (rt, sk, skb, fl.saddr, fl.daddr,0 ,ip4_dst_hoplimit (&rt->dst),0 , fl.fl4_sport, - fl.fl4_dport, false, sk->sk_no_check_tx); + fl.fl4_dport, false, sk->sk_no_check_tx,0 ); ret =0 ; err:local_bh_enable ();
drivers/net/vxlan/vxlan_core.c +1 -1
@@ -2522,7 +2522,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,udp_tunnel_xmit_skb (rt, sock4->sock->sk, skb, saddr, pkey->u.ipv4.dst, tos, ttl, df, - src_port, dst_port, xnet, !udp_sum); + src_port, dst_port, xnet, !udp_sum,0 );#if IS_ENABLED (CONFIG_IPV6 ) } else {struct vxlan_sock *sock6 =rcu_dereference (vxlan->vn6_sock);
drivers/net/wireguard/socket.c +1 -1
@@ -84,7 +84,7 @@ static int send4(struct wg_device *wg, struct sk_buff *skb, skb->ignore_df =1 ;udp_tunnel_xmit_skb (rt, sock, skb, fl.saddr, fl.daddr, ds,ip4_dst_hoplimit (&rt->dst),0 , fl.fl4_sport, - fl.fl4_dport, false, false); + fl.fl4_dport, false, false,0 ); goto out; err:
include/net/ip_tunnels.h +1 -1
@@ -603,7 +603,7 @@ static inline int iptunnel_pull_header(struct sk_buff *skb, int hdr_len,void iptunnel_xmit (struct sock *sk,struct rtable *rt,struct sk_buff *skb,__be32 src,__be32 dst,u8 proto, -u8 tos,u8 ttl,__be16 df,bool xnet); +u8 tos,u8 ttl,__be16 df,bool xnet,u16 ipcb_flags);struct metadata_dst *iptunnel_metadata_reply (struct metadata_dst *md,gfp_t flags); int skb_tunnel_check_pmtu(struct sk_buff *skb, struct dst_entry *encap_dst,
include/net/udp_tunnel.h +1 -1
@@ -150,7 +150,7 @@ static inline void udp_tunnel_drop_rx_info(struct net_device *dev)void udp_tunnel_xmit_skb (struct rtable *rt,struct sock *sk,struct sk_buff *skb,__be32 src,__be32 dst,__u8 tos,__u8 ttl,__be16 df,__be16 src_port,__be16 dst_port, -bool xnet,bool nocheck); +bool xnet,bool nocheck,u16 ipcb_flags); int udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk, struct sk_buff *skb,
net/ipv4/ip_tunnel.c +2 -2
@@ -668,7 +668,7 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,ip_tunnel_adj_headroom (dev, headroom);iptunnel_xmit (NULL , rt, skb, fl4.saddr, fl4.daddr, proto, tos, ttl, - df, !net_eq (tunnel->net,dev_net (dev))); + df, !net_eq (tunnel->net,dev_net (dev)),0 );return ; tx_error:DEV_STATS_INC (dev, tx_errors);
@@ -857,7 +857,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,ip_tunnel_adj_headroom (dev, max_headroom);iptunnel_xmit (NULL , rt, skb, fl4.saddr, fl4.daddr, protocol, tos, ttl, - df, !net_eq (tunnel->net,dev_net (dev))); + df, !net_eq (tunnel->net,dev_net (dev)),0 );return ;#if IS_ENABLED (CONFIG_IPV6 )
net/ipv4/ip_tunnel_core.c +3 -1
@@ -49,7 +49,8 @@ EXPORT_SYMBOL(ip6tun_encaps);void iptunnel_xmit (struct sock *sk,struct rtable *rt,struct sk_buff *skb,__be32 src,__be32 dst,__u8 proto, -__u8 tos,__u8 ttl,__be16 df,bool xnet) +__u8 tos,__u8 ttl,__be16 df,bool xnet, +u16 ipcb_flags) {int pkt_len = skb->len -skb_inner_network_offset (skb);struct net *net =dev_net (rt->dst.dev);
@@ -62,6 +63,7 @@ void iptunnel_xmit(struct sock *sk, struct rtable *rt, struct sk_buff *skb,skb_clear_hash_if_not_l4 (skb);skb_dst_set (skb, &rt->dst);memset (IPCB (skb),0 ,sizeof (*IPCB (skb))); +IPCB (skb)->flags = ipcb_flags;/* Push down and install the IP header. */ skb_push (skb,sizeof (struct iphdr ));
net/ipv4/udp_tunnel_core.c +3 -2
@@ -169,7 +169,7 @@ EXPORT_SYMBOL_GPL(udp_tunnel_notify_del_rx_port);void udp_tunnel_xmit_skb (struct rtable *rt,struct sock *sk,struct sk_buff *skb,__be32 src,__be32 dst,__u8 tos,__u8 ttl,__be16 df,__be16 src_port,__be16 dst_port, -bool xnet,bool nocheck) +bool xnet,bool nocheck,u16 ipcb_flags) {struct udphdr *uh;
@@ -185,7 +185,8 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skbudp_set_csum (nocheck, skb, src, dst, skb->len); -iptunnel_xmit (sk, rt, skb, src, dst,IPPROTO_UDP , tos, ttl, df, xnet); +iptunnel_xmit (sk, rt, skb, src, dst,IPPROTO_UDP , tos, ttl, df, xnet, + ipcb_flags); }EXPORT_SYMBOL_GPL (udp_tunnel_xmit_skb);
net/ipv6/sit.c +1 -1
@@ -1035,7 +1035,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,skb_set_inner_ipproto (skb,IPPROTO_IPV6 );iptunnel_xmit (NULL , rt, skb, fl4.saddr, fl4.daddr, protocol, tos, ttl, - df, !net_eq (tunnel->net,dev_net (dev))); + df, !net_eq (tunnel->net,dev_net (dev)),0 );return NETDEV_TX_OK ; tx_error_icmp:
net/sctp/protocol.c +2 -1
@@ -1103,7 +1103,8 @@ static inline int sctp_v4_xmit(struct sk_buff *skb, struct sctp_transport *t)skb_set_inner_ipproto (skb,IPPROTO_SCTP );udp_tunnel_xmit_skb (dst_rtable (dst), sk, skb, fl4->saddr, fl4->daddr, dscp,ip4_dst_hoplimit (dst), df, -sctp_sk (sk)->udp_port, t->encap_port, false, false); +sctp_sk (sk)->udp_port, t->encap_port, false, false, +0 );return 0 ; }
net/tipc/udp_media.c +1 -1
@@ -197,7 +197,7 @@ static int tipc_udp_xmit(struct net *net, struct sk_buff *skb, ttl =ip4_dst_hoplimit (&rt->dst);udp_tunnel_xmit_skb (rt, ub->ubsock->sk, skb, src->ipv4.s_addr, dst->ipv4.s_addr,0 , ttl,0 , src->port, - dst->port, false, true); + dst->port, false, true,0 );#if IS_ENABLED (CONFIG_IPV6 ) } else {if (!ndst) {
Signature · 1 line
2.49.0
Jakub Kicinski
maintainerRe: [PATCH net-next v2 01/14] net: ipv4: Add a flags argument to iptunnel_xmit(), udp_tunnel_xmit_skb()
net/ipv4/udp_tunnel_core.c EXPORT_SYMBOL_GPL(udp_tunnel_notify_del_rx_port); · line 172
Quoted 5 lines · On Thu, 12 Jun 2025 22:10:35 +0200 Petr Machata wrote:
void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct
sk_buff*skb,net/ipv4/udp_tunnel_core.c +1
-1__be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df, __be16 src_port, __be16 dst_port, - bool xnet, bool nocheck) + bool xnet, bool nocheck, u16 ipcb_flags)
This is a lot of arguments for a function.
I don't have a great suggestion off the top of my head, but maybe think more about it?
Re: [PATCH net-next v2 01/14] net: ipv4: Add a flags argument to iptunnel_xmit(), udp_tunnel_xmit_skb()
Quoted 10 lines · Jakub Kicinski
net/ipv4/udp_tunnel_core.c EXPORT_SYMBOL_GPL(udp_tunnel_notify_del_rx_port); · line 172
Quoted 5 lines · On Thu, 12 Jun 2025 22:10:35 +0200 Petr Machata wrote:
void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct
sk_buff*skb,net/ipv4/udp_tunnel_core.c +1
-1__be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df, __be16 src_port, __be16 dst_port, - bool xnet, bool nocheck) + bool xnet, bool nocheck, u16 ipcb_flags)This is a lot of arguments for a function.
I don't have a great suggestion off the top of my head, but maybe think more about it?
It wraps functions that take many arguments ^o^
We could exchange src_port, dst_port by passing in the UDP header directly, but I don't think that's a good idea.
I guess I don't have great ideas either.
PATCH v2 2/14 net: ipv4: ipmr: ipmr_queue_xmit(): Drop local variable `dev'
In reply to Petr MachataThe variable is used for caching of rt->dst.dev. The netdevice referenced therein does not change during the scope of validity of that local. At the same time, the local is only used twice, and each of these uses will end up in a different function in the following patches, further eliminating any use the local could have had.
Drop the local altogether and inline the uses.
--- net/ipv4/ipmr.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
net/ipv4/ipmr.c +2 -5
@@ -1859,7 +1859,6 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,const struct iphdr *iph =ip_hdr (skb);struct vif_device *vif = &mrt->vif_table[vifi];struct net_device *vif_dev; -struct net_device *dev;struct rtable *rt;struct flowi4 fl4;int encap =0 ;
@@ -1898,8 +1897,6 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt, goto out_free; } - dev = rt->dst.dev; -if (skb->len+encap >dst_mtu (&rt->dst) && (ntohs (iph->frag_off) &IP_DF )) {/* Do not fragment multicasts. Alas, IPv4 does not * allow to send ICMP, so that packets will disappear
@@ -1910,7 +1907,7 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt, goto out_free; } - encap +=LL_RESERVED_SPACE (dev) + rt->dst.header_len; + encap +=LL_RESERVED_SPACE (rt->dst.dev) + rt->dst.header_len;if (skb_cow (skb, encap)) {ip_rt_put (rt);
@@ -1947,7 +1944,7 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,* result in receiving multiple packets. */ NF_HOOK (NFPROTO_IPV4 ,NF_INET_FORWARD , - net,NULL , skb, skb->dev, dev, + net,NULL , skb, skb->dev, rt->dst.dev, ipmr_forward_finish);return ;
Signature · 1 line
2.49.0
PATCH v2 3/14 net: ipv4: ipmr: Split ipmr_queue_xmit() in two
In reply to Petr MachataSome of the work of ipmr_queue_xmit() is specific to IPMR forwarding, and should not take place on the output path. In order to allow reuse of the common parts, split the function into two: the ipmr_prepare_xmit() helper that takes care of the common bits, and the ipmr_queue_fwd_xmit(), which invokes the former and encapsulates the whole forwarding algorithm.
--- net/ipv4/ipmr.c | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-)
net/ipv4/ipmr.c +29 -16
@@ -1853,8 +1853,8 @@ static bool ipmr_forward_offloaded(struct sk_buff *skb, struct mr_table *mrt,/* Processing handlers for ipmr_forward, under rcu_read_lock() */ -static void ipmr_queue_xmit (struct net *net,struct mr_table *mrt, -int in_vifi,struct sk_buff *skb,int vifi) +static int ipmr_prepare_xmit (struct net *net,struct mr_table *mrt, +struct sk_buff *skb,int vifi) {const struct iphdr *iph =ip_hdr (skb);struct vif_device *vif = &mrt->vif_table[vifi];
@@ -1865,7 +1865,7 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt, vif_dev =vif_dev_read (vif);if (!vif_dev) - goto out_free; +return -1 ;if (vif->flags &VIFF_REGISTER ) {WRITE_ONCE (vif->pkt_out, vif->pkt_out +1 );
@@ -1873,12 +1873,9 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,DEV_STATS_ADD (vif_dev, tx_bytes, skb->len);DEV_STATS_INC (vif_dev, tx_packets);ipmr_cache_report (mrt, skb, vifi,IGMPMSG_WHOLEPKT ); - goto out_free; +return -1 ; } -if (ipmr_forward_offloaded (skb, mrt, in_vifi, vifi)) - goto out_free; -if (vif->flags &VIFF_TUNNEL ) { rt = ip_route_output_ports(net, &fl4, NULL, vif->remote, vif->local,
@@ -1886,7 +1883,7 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,IPPROTO_IPIP , iph->tos & INET_DSCP_MASK, vif->link);if (IS_ERR (rt)) - goto out_free; +return -1 ; encap =sizeof (struct iphdr ); }else { rt = ip_route_output_ports(net, &fl4, NULL, iph->daddr, 0,
@@ -1894,7 +1891,7 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,IPPROTO_IPIP , iph->tos & INET_DSCP_MASK, vif->link);if (IS_ERR (rt)) - goto out_free; +return -1 ; }if (skb->len+encap >dst_mtu (&rt->dst) && (ntohs (iph->frag_off) &IP_DF )) {
@@ -1904,14 +1901,14 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,*/ IP_INC_STATS (net,IPSTATS_MIB_FRAGFAILS );ip_rt_put (rt); - goto out_free; +return -1 ; } encap +=LL_RESERVED_SPACE (rt->dst.dev) + rt->dst.header_len;if (skb_cow (skb, encap)) {ip_rt_put (rt); - goto out_free; +return -1 ; }WRITE_ONCE (vif->pkt_out, vif->pkt_out +1 );
@@ -1931,6 +1928,22 @@ static void ipmr_queue_xmit(struct net *net, struct mr_table *mrt,DEV_STATS_ADD (vif_dev, tx_bytes, skb->len); } +return 0 ; +} + +static void ipmr_queue_fwd_xmit (struct net *net,struct mr_table *mrt, +int in_vifi,struct sk_buff *skb,int vifi) +{ +struct rtable *rt; + +if (ipmr_forward_offloaded (skb, mrt, in_vifi, vifi)) + goto out_free; + +if (ipmr_prepare_xmit (net, mrt, skb, vifi)) + goto out_free; + + rt =skb_rtable (skb); +IPCB (skb)->flags |=IPSKB_FORWARDED ;/* RFC1584 teaches, that DVMRP/PIM router must deliver packets locally
@@ -2062,8 +2075,8 @@ static void ip_mr_forward(struct net *net, struct mr_table *mrt,struct sk_buff *skb2 =skb_clone (skb,GFP_ATOMIC );if (skb2) -ipmr_queue_xmit (net, mrt, true_vifi, - skb2, psend); +ipmr_queue_fwd_xmit (net, mrt, true_vifi, + skb2, psend); } psend = ct; }
@@ -2074,10 +2087,10 @@ static void ip_mr_forward(struct net *net, struct mr_table *mrt,struct sk_buff *skb2 =skb_clone (skb,GFP_ATOMIC );if (skb2) -ipmr_queue_xmit (net, mrt, true_vifi, skb2, - psend); +ipmr_queue_fwd_xmit (net, mrt, true_vifi, skb2, + psend); }else { -ipmr_queue_xmit (net, mrt, true_vifi, skb, psend); +ipmr_queue_fwd_xmit (net, mrt, true_vifi, skb, psend);return ; } }
Signature · 1 line
2.49.0
PATCH v2 4/14 net: ipv4: Add ip_mr_output()
In reply to Petr MachataMulticast routing is today handled in the input path. Locally generated MC packets don't hit the IPMR code today. Thus if a VXLAN remote address is multicast, the driver needs to set an OIF during route lookup. Thus MC routing configuration needs to be kept in sync with the VXLAN FDB and MDB. Ideally, the VXLAN packets would be routed by the MC routing code instead.
To that end, this patch adds support to route locally generated multicast packets. The newly-added routines do largely what ip_mr_input() and ip_mr_forward() do: make an MR cache lookup to find where to send the packets, and use ip_mc_output() to send each of them. When no cache entry is found, the packet is punted to the daemon for resolution.
However, an installation that uses a VXLAN underlay netdevice for which it also has matching MC routes, would get a different routing with this patch. Previously, the MC packets would be delivered directly to the underlay port, whereas now they would be MC-routed. In order to avoid this change in behavior, introduce an IPCB flag. Only if the flag is set will ip_mr_output() actually engage, otherwise it reverts to ip_mc_output().
This code is based on work by Roopa Prabhu and Nikolay Aleksandrov.
--- include/net/ip.h | 2 + net/ipv4/ipmr.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++ net/ipv4/route.c | 2 +- 3 files changed, 120 insertions(+), 1 deletion(-)
include/net/ip.h +2 -0
@@ -59,6 +59,7 @@ struct inet_skb_parm {#define IPSKB_L3SLAVE BIT(7)#define IPSKB_NOPOLICY BIT(8)#define IPSKB_MULTIPATH BIT(9) +#define IPSKB_MCROUTE BIT(10)u16 frag_max_size; };
@@ -167,6 +168,7 @@ void ip_list_rcv(struct list_head *head, struct packet_type *pt,int ip_local_deliver (struct sk_buff *skb);void ip_protocol_deliver_rcu (struct net *net,struct sk_buff *skb,int proto);int ip_mr_input (struct sk_buff *skb); +int ip_mr_output (struct net *net,struct sock *sk,struct sk_buff *skb);int ip_output (struct net *net,struct sock *sk,struct sk_buff *skb);int ip_mc_output (struct net *net,struct sock *sk,struct sk_buff *skb); int ip_do_fragment(struct net *net, struct sock *sk, struct sk_buff *skb,
net/ipv4/ipmr.c +117 -0
129 lines not on this page · raw message
net/ipv4/route.c +1 -1
@@ -2660,7 +2660,7 @@ static struct rtable *__mkroute_output(const struct fib_result *res,if (IN_DEV_MFORWARD (in_dev) && !ipv4_is_local_multicast (fl4->daddr)) { rth->dst.input = ip_mr_input; - rth->dst.output = ip_mc_output; + rth->dst.output = ip_mr_output; } }#endif
Signature · 1 line
2.49.0
Jakub Kicinski
maintainerRe: [PATCH net-next v2 04/14] net: ipv4: Add ip_mr_output()
net/ipv4/ipmr.c int ip_mr_input(struct sk_buff *skb) · line 2255
Quoted 5 lines · On Thu, 12 Jun 2025 22:10:38 +0200 Petr Machata wrote:
net/ipv4/ipmr.c +5
-0+/* Forward the frame */ +if (c->mfc_origin ==htonl (INADDR_ANY ) && + c->mfc_mcastgrp ==htonl (INADDR_ANY )) { +if (ip_hdr (skb)->ttl > + c->_c.mfc_un.res.ttls[c->_c.mfc_parent]) {
weird indent?
net/ipv4/ipmr.c int ip_mr_input(struct sk_buff *skb) · line 2268
Quoted 13 lines
net/ipv4/ipmr.c +13
-0+/* It's an (*,*) entry and the packet is not coming from +* the upstream: forward the packet to the upstream +* only. +*/ + psend = c->_c.mfc_parent; + goto last_xmit; + } + goto dont_xmit; + } + +for (ct = c->_c.mfc_un.res.maxvif -1 ; + ct >= c->_c.mfc_un.res.minvif; ct--) { +if (ip_hdr (skb)->ttl > c->_c.mfc_un.res.ttls[ct]) {
I'd be tempted to invert condition, continue, save a level of indent. Presumably we expect TTL to actually be large enough so that'd also make the expected path not under the if ?
net/ipv4/ipmr.c int ip_mr_input(struct sk_buff *skb) · line 2273
Quoted 5 lines
net/ipv4/ipmr.c +5
-0+if (psend !=-1 ) { +struct sk_buff *skb2 =skb_clone (skb, +GFP_ATOMIC ); + +if (skb2)
maybe this is some local custom in this code but:
struct sk_buff *skb2; skb2 = skb_clone(skb, GFP_ATOMIC); if (skb2)
same LoC, less ugly.
Quoted 2 lines
net/ipv4/ipmr.c +2
-0+ipmr_queue_output_xmit (net, mrt, + skb2, psend);
Re: [PATCH net-next v2 04/14] net: ipv4: Add ip_mr_output()
Quoted 8 lines · Jakub Kicinski
net/ipv4/ipmr.c int ip_mr_input(struct sk_buff *skb) · line 2255
Quoted 5 lines · On Thu, 12 Jun 2025 22:10:38 +0200 Petr Machata wrote:
net/ipv4/ipmr.c +5
-0+/* Forward the frame */ +if (c->mfc_origin ==htonl (INADDR_ANY ) && + c->mfc_mcastgrp ==htonl (INADDR_ANY )) { +if (ip_hdr (skb)->ttl > + c->_c.mfc_un.res.ttls[c->_c.mfc_parent]) {weird indent?
I suppose one extra tab would have sufficed.
Quoted 9 lines
Quoted 9 lines
net/ipv4/ipmr.c +9
-0+/* It's an (*,*) entry and the packet is not coming from +* the upstream: forward the packet to the upstream +* only. +*/ + psend = c->_c.mfc_parent; + goto last_xmit; + } + goto dont_xmit; + }
Quoted 8 lines
net/ipv4/ipmr.c int ip_mr_input(struct sk_buff *skb) · line 2268
Quoted 4 lines
net/ipv4/ipmr.c +4
-0+ +for (ct = c->_c.mfc_un.res.maxvif -1 ; + ct >= c->_c.mfc_un.res.minvif; ct--) { +if (ip_hdr (skb)->ttl > c->_c.mfc_un.res.ttls[ct]) {I'd be tempted to invert condition, continue, save a level of indent. Presumably we expect TTL to actually be large enough so that'd also make the expected path not under the if ?
The TTL condition is expressed several times across the MR code, always in this way. It is the happy path, yeah, but I kept it like this for consistency reasons.
Quoted 8 lines
net/ipv4/ipmr.c int ip_mr_input(struct sk_buff *skb) · line 2273
Quoted 5 lines
net/ipv4/ipmr.c +5
-0+if (psend !=-1 ) { +struct sk_buff *skb2 =skb_clone (skb, +GFP_ATOMIC ); + +if (skb2)maybe this is some local custom in this code but:
Quoted 6 lines
struct sk_buff *skb2; skb2 = skb_clone(skb, GFP_ATOMIC); if (skb2)same LoC, less ugly.
OK.
Quoted 3 lines
Quoted 3 lines
net/ipv4/ipmr.c +2
-0+ipmr_queue_output_xmit (net, mrt, + skb2, psend);
PATCH v2 5/14 net: ipv6: Make udp_tunnel6_xmit_skb() void
In reply to Petr MachataThe function always returns zero, thus the return value does not carry any signal. Just make it void.
Most callers already ignore the return value. However:
- Refold arguments of the call from sctp_v6_xmit() so that they fit into the 80-column limit.
- tipc_udp_xmit() initializes err from the return value, but that should already be always zero at that point. So there's no practical change, but elision of the assignment prompts a couple more tweaks to clean up the function.
---
Notes:
CC:Marcelo Ricardo Leitner <[email protected]> CC:[email protected]
CC:Jon Maloy <[email protected]> CC:[email protected]
include/net/udp_tunnel.h | 14 +++++++------- net/ipv6/ip6_udp_tunnel.c | 15 +++++++-------- net/sctp/ipv6.c | 7 ++++--- net/tipc/udp_media.c | 10 +++++----- 4 files changed, 23 insertions(+), 23 deletions(-)
include/net/udp_tunnel.h +7 -7
@@ -152,13 +152,13 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb __be16 df, __be16 src_port, __be16 dst_port, bool xnet, bool nocheck, u16 ipcb_flags); -int udp_tunnel6_xmit_skb (struct dst_entry *dst,struct sock *sk, -struct sk_buff *skb, -struct net_device *dev, -const struct in6_addr *saddr, -const struct in6_addr *daddr, -__u8 prio,__u8 ttl,__be32 label, -__be16 src_port,__be16 dst_port,bool nocheck); +void udp_tunnel6_xmit_skb (struct dst_entry *dst,struct sock *sk, +struct sk_buff *skb, +struct net_device *dev, +const struct in6_addr *saddr, +const struct in6_addr *daddr, +__u8 prio,__u8 ttl,__be32 label, +__be16 src_port,__be16 dst_port,bool nocheck);void udp_tunnel_sock_release (struct socket *sock);
net/ipv6/ip6_udp_tunnel.c +7 -8
@@ -74,13 +74,13 @@ int udp_sock_create6(struct net *net, struct udp_port_cfg *cfg, }EXPORT_SYMBOL_GPL (udp_sock_create6); -int udp_tunnel6_xmit_skb (struct dst_entry *dst,struct sock *sk, -struct sk_buff *skb, -struct net_device *dev, -const struct in6_addr *saddr, -const struct in6_addr *daddr, -__u8 prio,__u8 ttl,__be32 label, -__be16 src_port,__be16 dst_port,bool nocheck) +void udp_tunnel6_xmit_skb (struct dst_entry *dst,struct sock *sk, +struct sk_buff *skb, +struct net_device *dev, +const struct in6_addr *saddr, +const struct in6_addr *daddr, +__u8 prio,__u8 ttl,__be32 label, +__be16 src_port,__be16 dst_port,bool nocheck) {struct udphdr *uh;struct ipv6hdr *ip6h;
@@ -109,7 +109,6 @@ int udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk, ip6h->saddr = *saddr;ip6tunnel_xmit (sk, skb, dev); -return 0 ; }EXPORT_SYMBOL_GPL (udp_tunnel6_xmit_skb);
net/sctp/ipv6.c +4 -3
@@ -261,9 +261,10 @@ static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *t)skb_set_inner_ipproto (skb,IPPROTO_SCTP ); label =ip6_make_flowlabel (sock_net (sk), skb, fl6->flowlabel, true, fl6); -return udp_tunnel6_xmit_skb (dst, sk, skb,NULL , &fl6->saddr, - &fl6->daddr, tclass,ip6_dst_hoplimit (dst), - label,sctp_sk (sk)->udp_port, t->encap_port, false); +udp_tunnel6_xmit_skb (dst, sk, skb,NULL , &fl6->saddr, &fl6->daddr, + tclass,ip6_dst_hoplimit (dst), label, +sctp_sk (sk)->udp_port, t->encap_port, false); +return 0 ; }/* Returns the dst cache entry for the given source and destination ip
net/tipc/udp_media.c +5 -5
@@ -172,7 +172,7 @@ static int tipc_udp_xmit(struct net *net, struct sk_buff *skb, struct udp_media_addr *dst, struct dst_cache *cache) { struct dst_entry *ndst; -int ttl, err =0 ; +int ttl, err;local_bh_disable (); ndst =dst_cache_get (cache);
@@ -217,13 +217,13 @@ static int tipc_udp_xmit(struct net *net, struct sk_buff *skb, dst_cache_set_ip6(cache, ndst, &fl6.saddr); } ttl =ip6_dst_hoplimit (ndst); - err =udp_tunnel6_xmit_skb (ndst, ub->ubsock->sk, skb,NULL , - &src->ipv6, &dst->ipv6,0 , ttl,0 , - src->port, dst->port, false); +udp_tunnel6_xmit_skb (ndst, ub->ubsock->sk, skb,NULL , + &src->ipv6, &dst->ipv6,0 , ttl,0 , + src->port, dst->port, false);#endif }local_bh_enable (); -return err; +return 0 ; tx_error:local_bh_enable ();
Signature · 1 line
2.49.0
PATCH v2 6/14 net: ipv6: Add a flags argument to ip6tunnel_xmit(), udp_tunnel6_xmit_skb()
In reply to Petr Machataip6tunnel_xmit() erases the contents of the SKB control block. In order to be able to set particular IP6CB flags on the SKB, add a corresponding parameter, and propagate it to udp_tunnel6_xmit_skb() as well.
In one of the following patches, VXLAN driver will use this facility to mark packets as subject to IPv6 multicast routing.
---
Notes:
CC:Pablo Neira Ayuso <[email protected]> CC:[email protected] CC:Andrew Lunn <[email protected]> CC:Antonio Quartulli <[email protected]> CC:"Jason A. Donenfeld" <[email protected]> CC:[email protected]
CC:Marcelo Ricardo Leitner <[email protected]> CC:[email protected]
CC:Jon Maloy <[email protected]> CC:[email protected]
drivers/net/bareudp.c | 3 ++- drivers/net/geneve.c | 3 ++- drivers/net/gtp.c | 2 +- drivers/net/ovpn/udp.c | 2 +- drivers/net/vxlan/vxlan_core.c | 3 ++- drivers/net/wireguard/socket.c | 2 +- include/net/ip6_tunnel.h | 3 ++- include/net/udp_tunnel.h | 3 ++- net/ipv6/ip6_tunnel.c | 2 +- net/ipv6/ip6_udp_tunnel.c | 5 +++-- net/sctp/ipv6.c | 2 +- net/tipc/udp_media.c | 2 +- 12 files changed, 19 insertions(+), 13 deletions(-)
drivers/net/bareudp.c +2 -1
@@ -431,7 +431,8 @@ static int bareudp6_xmit_skb(struct sk_buff *skb, struct net_device *dev, &saddr, &daddr, prio, ttl, info->key.label, sport, bareudp->port, !test_bit (IP_TUNNEL_CSUM_BIT , - info->key.tun_flags)); + info->key.tun_flags), + 0);return 0 ; free_dst:
drivers/net/geneve.c +2 -1
@@ -1014,7 +1014,8 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev, &saddr, &key->u.ipv6.dst, prio, ttl, info->key.label, sport, geneve->cfg.info.key.tp_dst, !test_bit (IP_TUNNEL_CSUM_BIT , - info->key.tun_flags)); + info->key.tun_flags), + 0); return 0; }#endif
drivers/net/gtp.c +1 -1
@@ -1316,7 +1316,7 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)ip6_dst_hoplimit (&pktinfo.rt->dst),0 , pktinfo.gtph_port, pktinfo.gtph_port, - false); + false, 0);#else goto tx_err;#endif
drivers/net/ovpn/udp.c +1 -1
@@ -274,7 +274,7 @@ static int ovpn_udp6_output(struct ovpn_peer *peer, struct ovpn_bind *bind, skb->ignore_df =1 ;udp_tunnel6_xmit_skb (dst, sk, skb, skb->dev, &fl.saddr, &fl.daddr,0 ,ip6_dst_hoplimit (dst),0 , fl.fl6_sport, - fl.fl6_dport,udp_get_no_check6_tx (sk)); + fl.fl6_dport,udp_get_no_check6_tx (sk),0 ); ret =0 ; err:local_bh_enable ();
drivers/net/vxlan/vxlan_core.c +2 -1
@@ -2586,7 +2586,8 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,udp_tunnel6_xmit_skb (ndst, sock6->sock->sk, skb, dev, &saddr, &pkey->u.ipv6.dst, tos, ttl, - pkey->label, src_port, dst_port, !udp_sum); + pkey->label, src_port, dst_port, !udp_sum, +0 );#endif }vxlan_vnifilter_count (vxlan, vni,NULL ,VXLAN_VNI_STATS_TX , pkt_len);
drivers/net/wireguard/socket.c +1 -1
@@ -151,7 +151,7 @@ static int send6(struct wg_device *wg, struct sk_buff *skb, skb->ignore_df =1 ;udp_tunnel6_xmit_skb (dst, sock, skb, skb->dev, &fl.saddr, &fl.daddr, ds,ip6_dst_hoplimit (dst),0 , fl.fl6_sport, - fl.fl6_dport, false); + fl.fl6_dport, false,0 ); goto out; err:
include/net/ip6_tunnel.h +2 -1
@@ -152,11 +152,12 @@ int ip6_tnl_get_iflink(const struct net_device *dev);int ip6_tnl_change_mtu (struct net_device *dev,int new_mtu);static inline void ip6tunnel_xmit (struct sock *sk,struct sk_buff *skb, -struct net_device *dev) +struct net_device *dev,u16 ip6cb_flags) {int pkt_len, err;memset (skb->cb,0 ,sizeof (struct inet6_skb_parm )); +IP6CB (skb)->flags = ip6cb_flags; pkt_len = skb->len -skb_inner_network_offset (skb); err =ip6_local_out (dev_net (skb_dst (skb)->dev), sk, skb);
include/net/udp_tunnel.h +2 -1
@@ -158,7 +158,8 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,const struct in6_addr *saddr, const struct in6_addr *daddr, __u8 prio, __u8 ttl, __be32 label, - __be16 src_port, __be16 dst_port, bool nocheck); + __be16 src_port, __be16 dst_port, bool nocheck, + u16 ip6cb_flags);void udp_tunnel_sock_release (struct socket *sock);
net/ipv6/ip6_tunnel.c +1 -1
@@ -1278,7 +1278,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield, ipv6h->nexthdr = proto; ipv6h->saddr = fl6->saddr; ipv6h->daddr = fl6->daddr; -ip6tunnel_xmit (NULL , skb, dev); +ip6tunnel_xmit (NULL , skb, dev,0 );return 0 ; tx_err_link_failure:DEV_STATS_INC (dev, tx_carrier_errors);
net/ipv6/ip6_udp_tunnel.c +3 -2
@@ -80,7 +80,8 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,const struct in6_addr *saddr, const struct in6_addr *daddr, __u8 prio, __u8 ttl, __be32 label, - __be16 src_port, __be16 dst_port, bool nocheck) + __be16 src_port, __be16 dst_port, bool nocheck, + u16 ip6cb_flags) { struct udphdr *uh;struct ipv6hdr *ip6h;
@@ -108,7 +109,7 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk, ip6h->daddr = *daddr; ip6h->saddr = *saddr; -ip6tunnel_xmit (sk, skb, dev); +ip6tunnel_xmit (sk, skb, dev, ip6cb_flags); }EXPORT_SYMBOL_GPL (udp_tunnel6_xmit_skb);
net/sctp/ipv6.c +1 -1
@@ -263,7 +263,7 @@ static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *t)udp_tunnel6_xmit_skb (dst, sk, skb,NULL , &fl6->saddr, &fl6->daddr, tclass,ip6_dst_hoplimit (dst), label, -sctp_sk (sk)->udp_port, t->encap_port, false); +sctp_sk (sk)->udp_port, t->encap_port, false,0 );return 0 ; }
net/tipc/udp_media.c +1 -1
@@ -219,7 +219,7 @@ static int tipc_udp_xmit(struct net *net, struct sk_buff *skb, ttl =ip6_dst_hoplimit (ndst);udp_tunnel6_xmit_skb (ndst, ub->ubsock->sk, skb,NULL , &src->ipv6, &dst->ipv6,0 , ttl,0 , - src->port, dst->port, false); + src->port, dst->port, false,0 );#endif }local_bh_enable ();
Signature · 1 line
2.49.0
PATCH v2 7/14 net: ipv6: ip6mr: Fix in/out netdev to pass to the FORWARD chain
In reply to Petr MachataThe netfilter hook is invoked with skb->dev for input netdevice, and vif_dev for output netdevice. However at the point of invocation, skb->dev is already set to vif_dev, and MR-forwarded packets are reported with in=out:
# ip6tables -A FORWARD -j LOG --log-prefix '[forw]' # cd tools/testing/selftests/net/forwarding # ./router_multicast.sh # dmesg | fgrep '[forw]' [ 1670.248245] [forw]IN=v5 OUT=v5 [...]
For reference, IPv4 MR code shows in and out as appropriate. Fix by caching skb->dev and using the updated value for output netdev.
---
Notes:
This never worked correctly, hence going through net-next.
---
CC: YOSHIFUJI Hideaki <[email protected]>
net/ipv6/ip6mr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv6/ip6mr.c +2 -1
@@ -2039,6 +2039,7 @@ static int ip6mr_forward2(struct net *net, struct mr_table *mrt, struct sk_buff *skb, int vifi) {struct vif_device *vif = &mrt->vif_table[vifi]; +struct net_device *indev = skb->dev;struct net_device *vif_dev;struct ipv6hdr *ipv6h;struct dst_entry *dst;
@@ -2101,7 +2102,7 @@ static int ip6mr_forward2(struct net *net, struct mr_table *mrt,IP6CB (skb)->flags |=IP6SKB_FORWARDED ;return NF_HOOK (NFPROTO_IPV6 ,NF_INET_FORWARD , - net,NULL , skb, skb->dev, vif_dev, + net,NULL , skb, indev, skb->dev, ip6mr_forward2_finish); out_free:
Signature · 1 line
2.49.0
PATCH v2 8/14 net: ipv6: ip6mr: Extract a helper out of ip6mr_forward2()
In reply to Petr MachataSome of the work of ip6mr_forward2() is specific to IPMR forwarding, and should not take place on the output path. In order to allow reuse of the common parts, extract out of the function a helper, ip6mr_prepare_forward().
--- net/ipv6/ip6mr.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
net/ipv6/ip6mr.c +16 -6
@@ -2035,11 +2035,10 @@ static inline int ip6mr_forward2_finish(struct net *net, struct sock *sk, struct* Processing handlers for ip6mr_forward */ -static int ip6mr_forward2 (struct net *net,struct mr_table *mrt, -struct sk_buff *skb,int vifi) +static int ip6mr_prepare_xmit (struct net *net,struct mr_table *mrt, +struct sk_buff *skb,int vifi) {struct vif_device *vif = &mrt->vif_table[vifi]; -struct net_device *indev = skb->dev;struct net_device *vif_dev;struct ipv6hdr *ipv6h;struct dst_entry *dst;
@@ -2098,6 +2097,20 @@ static int ip6mr_forward2(struct net *net, struct mr_table *mrt, ipv6h =ipv6_hdr (skb); ipv6h->hop_limit--; +return 0 ; + +out_free: +kfree_skb (skb); +return -1 ; +} + +static int ip6mr_forward2 (struct net *net,struct mr_table *mrt, +struct sk_buff *skb,int vifi) +{ +struct net_device *indev = skb->dev; + +if (ip6mr_prepare_xmit (net, mrt, skb, vifi)) +return 0 ;IP6CB (skb)->flags |=IP6SKB_FORWARDED ;
@@ -2105,9 +2118,6 @@ static int ip6mr_forward2(struct net *net, struct mr_table *mrt, net,NULL , skb, indev, skb->dev, ip6mr_forward2_finish); -out_free: -kfree_skb (skb); -return 0 ; }/* Called with rcu_read_lock() */
Signature · 1 line
2.49.0
Jakub Kicinski
maintainerRe: [PATCH net-next v2 08/14] net: ipv6: ip6mr: Extract a helper out of ip6mr_forward2()
Quoted 11 lines · On Thu, 12 Jun 2025 22:10:42 +0200 Petr Machata wrote:
net/ipv6/ip6mr.c +2
-3-static int ip6mr_forward2 (struct net *net,struct mr_table *mrt, -struct sk_buff *skb,int vifi) +static int ip6mr_prepare_xmit (struct net *net,struct mr_table *mrt, +struct sk_buff *skb,int vifi) {struct vif_device *vif = &mrt->vif_table[vifi]; -struct net_device *indev = skb->dev;struct net_device *vif_dev;struct ipv6hdr *ipv6h;struct dst_entry *dst;
net/ipv6/ip6mr.c static int ip6mr_forward2(struct net *net, struct mr_table *mrt, · line 2105
Quoted 9 lines
net/ipv6/ip6mr.c +6
-0@@ -2098,6 +2097,20 @@ static int ip6mr_forward2(struct net *net, struct mr_table *mrt, ipv6h =ipv6_hdr (skb); ipv6h->hop_limit--; +return 0 ; + +out_free: +kfree_skb (skb); +return -1 ; +}
ipmr_prepare_xmit() does not free the skb on error, and ip6mr_prepare_xmit() does. The v6 version does lead to slightly cleaner code, but I wonder if its worth the asymmetry ?
Re: [PATCH net-next v2 08/14] net: ipv6: ip6mr: Extract a helper out of ip6mr_forward2()
Quoted 13 lines · Jakub Kicinski
Quoted 12 lines · On Thu, 12 Jun 2025 22:10:42 +0200 Petr Machata wrote:
net/ipv6/ip6mr.c +2
-3-static int ip6mr_forward2 (struct net *net,struct mr_table *mrt, -struct sk_buff *skb,int vifi) +static int ip6mr_prepare_xmit (struct net *net,struct mr_table *mrt, +struct sk_buff *skb,int vifi) {struct vif_device *vif = &mrt->vif_table[vifi]; -struct net_device *indev = skb->dev;struct net_device *vif_dev;struct ipv6hdr *ipv6h;struct dst_entry *dst;@@ -2098,6 +2097,20 @@ static int ip6mr_forward2(struct net *net, struct mr_table *mrt,
Quoted 12 lines
net/ipv6/ip6mr.c static int ip6mr_forward2(struct net *net, struct mr_table *mrt, · line 2105
Quoted 8 lines
net/ipv6/ip6mr.c +6
-0ipv6h =ipv6_hdr (skb); ipv6h->hop_limit--; +return 0 ; + +out_free: +kfree_skb (skb); +return -1 ; +}
ipmr_prepare_xmit()does not free the skb on error, andip6mr_prepare_xmit()does. The v6 version does lead to slightly cleaner code, but I wonder if its worth the asymmetry ?
Hmm, yeah, I think I had them symmetrical originally, but the ipmr_forward_offloaded() call didn't make sense in ipmr_prepare_xmit(), and pulled the cleanup up as it went out to the caller.
I'll do the same in IPv6, it makes sense to keep them similar.
PATCH v2 9/14 net: ipv6: Add ip6_mr_output()
In reply to Petr MachataMulticast routing is today handled in the input path. Locally generated MC packets don't hit the IPMR code today. Thus if a VXLAN remote address is multicast, the driver needs to set an OIF during route lookup. Thus MC routing configuration needs to be kept in sync with the VXLAN FDB and MDB. Ideally, the VXLAN packets would be routed by the MC routing code instead.
To that end, this patch adds support to route locally generated multicast packets. The newly-added routines do largely what ip6_mr_input() and ip6_mr_forward() do: make an MR cache lookup to find where to send the packets, and use ip6_output() to send each of them. When no cache entry is found, the packet is punted to the daemon for resolution.
Similarly to the IPv4 case in a previous patch, the new logic is contingent on a newly-added IP6CB flag being set.
--- include/linux/ipv6.h | 1 + include/linux/mroute6.h | 7 +++ net/ipv6/ip6mr.c | 114 ++++++++++++++++++++++++++++++++++++++++ net/ipv6/route.c | 1 + 4 files changed, 123 insertions(+)
include/linux/ipv6.h +1 -0
@@ -156,6 +156,7 @@ struct inet6_skb_parm {#define IP6SKB_SEG6 256#define IP6SKB_FAKEJUMBO 512#define IP6SKB_MULTIPATH 1024 +#define IP6SKB_MCROUTE 2048 };#if defined(CONFIG_NET_L3_MASTER_DEV )
include/linux/mroute6.h +7 -0
@@ -31,6 +31,7 @@ extern int ip6_mroute_getsockopt(struct sock *, int, sockptr_t, sockptr_t);extern int ip6_mr_input (struct sk_buff *skb);extern int ip6mr_compat_ioctl (struct sock *sk,unsigned int cmd,void __user *arg);extern int ip6_mr_init (void ); +extern int ip6_mr_output (struct net *net,struct sock *sk,struct sk_buff *skb);extern void ip6_mr_cleanup (void );int ip6mr_ioctl (struct sock *sk,int cmd,void *arg);#else
@@ -58,6 +59,12 @@ static inline int ip6_mr_init(void)return 0 ; } +static inline int +ip6_mr_output (struct net *net,struct sock *sk,struct sk_buff *skb) +{ +return 0 ; +} +static inline void ip6_mr_cleanup (void ) {return ;
net/ipv6/ip6mr.c +114 -0
132 lines not on this page · raw message
net/ipv6/route.c +1 -0
@@ -1145,6 +1145,7 @@ static void ip6_rt_init_dst(struct rt6_info *rt, const struct fib6_result *res) rt->dst.input = ip6_input; }else if (ipv6_addr_type (&f6i->fib6_dst.addr) &IPV6_ADDR_MULTICAST ) { rt->dst.input = ip6_mc_input; + rt->dst.output = ip6_mr_output; }else { rt->dst.input = ip6_forward; }
Signature · 1 line
2.49.0
Jakub Kicinski
maintainerRe: [PATCH net-next v2 09/14] net: ipv6: Add ip6_mr_output()
include/linux/mroute6.h static inline int ip6_mr_init(void) · line 66
Quoted 5 lines · On Thu, 12 Jun 2025 22:10:43 +0200 Petr Machata wrote:
include/linux/mroute6.h +5
-0+static inline int +ip6_mr_output (struct net *net,struct sock *sk,struct sk_buff *skb) +{ +return 0 ; +}
Shouldn't this free the skb?
That would explain why you're not seeing the problem our netdevsim runner doesn't have IPV6_MROUTE set, and you probably do?
Now that I found the bug in your code I will give you some extra comments :P
Re: [PATCH net-next v2 09/14] net: ipv6: Add ip6_mr_output()
Quoted 11 lines · Jakub Kicinski
include/linux/mroute6.h static inline int ip6_mr_init(void) · line 66
Quoted 5 lines · On Thu, 12 Jun 2025 22:10:43 +0200 Petr Machata wrote:
include/linux/mroute6.h +5
-0+static inline int +ip6_mr_output (struct net *net,struct sock *sk,struct sk_buff *skb) +{ +return 0 ; +}Shouldn't this free the skb?
That would explain why you're not seeing the problem our netdevsim runner doesn't have
IPV6_MROUTEset, and you probably do?
Yep, that's what reproduces it. The NIPA how-to-reproduce instructions show they use tools/testing/selftests/net/forwarding/config, but those enable IPV6_MROUTE, hence why it didn't reproduce for me.
This ip6_mr_output() fallback is completely broken, it should be calling ip6_output() obviously instead of silently leaking the packet :-|
Jakub Kicinski
maintainerRe: [PATCH net-next v2 09/14] net: ipv6: Add ip6_mr_output()
Quoted 6 lines · On Fri, 13 Jun 2025 20:59:27 +0200 Petr Machata wrote:
Quoted 2 lines
That would explain why you're not seeing the problem our netdevsim runner doesn't have
IPV6_MROUTEset, and you probably do?Yep, that's what reproduces it. The NIPA how-to-reproduce instructions show they use
tools/testing/selftests/net/forwarding/config, but those enableIPV6_MROUTE, hence why it didn't reproduce for me.
I see. I think the wiki tries to mention that only local config is used. I will add a couple of sentences to emphasize this.
PATCH v2 10/14 vxlan: Support MC routing in the underlay
In reply to Petr MachataLocally-generated MC packets have so far not been subject to MC routing. Instead an MC-enabled installation would maintain the MC routing tables, and separately from that the list of interfaces to send packets to as part of the VXLAN FDB and MDB.
In a previous patch, a ip_mr_output() and ip6_mr_output() routines were added for IPv4 and IPv6. All locally generated MC traffic is now passed through these functions. For reasons of backward compatibility, an SKB (IPCB / IP6CB) flag guards the actual MC routing.
This patch adds logic to set the flag, and the UAPI to enable the behavior.
---
Notes:
CC:Andrew Lunn <[email protected]>
CC:Menglong Dong <[email protected]>
drivers/net/vxlan/vxlan_core.c | 22 ++++++++++++++++++++-- include/net/vxlan.h | 5 ++++- include/uapi/linux/if_link.h | 1 + 3 files changed, 25 insertions(+), 3 deletions(-)
drivers/net/vxlan/vxlan_core.c +20 -2
@@ -2451,6 +2451,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,rcu_read_lock ();if (addr_family ==AF_INET ) {struct vxlan_sock *sock4 =rcu_dereference (vxlan->vn4_sock); +u16 ipcb_flags =0 ;struct rtable *rt;__be16 df =0 ;__be32 saddr;
@@ -2467,6 +2468,9 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev, goto tx_error; } +if (flags &VXLAN_F_MC_ROUTE ) + ipcb_flags |=IPSKB_MCROUTE ; +if (!info) {/* Bypass encapsulation if the destination is local */ err = encap_bypass_if_local(skb, dev, vxlan, AF_INET,
@@ -2522,11 +2526,13 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,udp_tunnel_xmit_skb (rt, sock4->sock->sk, skb, saddr, pkey->u.ipv4.dst, tos, ttl, df, - src_port, dst_port, xnet, !udp_sum,0 ); + src_port, dst_port, xnet, !udp_sum, + ipcb_flags);#if IS_ENABLED (CONFIG_IPV6 ) } else {struct vxlan_sock *sock6 =rcu_dereference (vxlan->vn6_sock);struct in6_addr saddr; +u16 ip6cb_flags =0 ;if (!ifindex) ifindex = sock6->sock->sk->sk_bound_dev_if;
@@ -2542,6 +2548,9 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev, goto tx_error; } +if (flags &VXLAN_F_MC_ROUTE ) + ip6cb_flags |=IP6SKB_MCROUTE ; +if (!info) {u32 rt6i_flags =dst_rt6_info (ndst)->rt6i_flags;
@@ -2587,7 +2596,7 @@ void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,udp_tunnel6_xmit_skb (ndst, sock6->sock->sk, skb, dev, &saddr, &pkey->u.ipv6.dst, tos, ttl, pkey->label, src_port, dst_port, !udp_sum, -0 ); + ip6cb_flags);#endif }vxlan_vnifilter_count (vxlan, vni,NULL ,VXLAN_VNI_STATS_TX , pkt_len);
@@ -3402,6 +3411,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = { [IFLA_VXLAN_LOCALBYPASS] = NLA_POLICY_MAX(NLA_U8, 1), [IFLA_VXLAN_LABEL_POLICY] = NLA_POLICY_MAX(NLA_U32, VXLAN_LABEL_MAX), [IFLA_VXLAN_RESERVED_BITS] = NLA_POLICY_EXACT_LEN(sizeof(struct vxlanhdr)), + [IFLA_VXLAN_MC_ROUTE] = NLA_POLICY_MAX(NLA_U8, 1), }; static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[],
@@ -4315,6 +4325,14 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],return err; } +if (data[IFLA_VXLAN_MC_ROUTE ]) { + err =vxlan_nl2flag (conf, data,IFLA_VXLAN_MC_ROUTE , +VXLAN_F_MC_ROUTE , changelink, + true, extack); +if (err) +return err; + } +if (tb[IFLA_MTU ]) {if (changelink) { NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU],
include/net/vxlan.h +4 -1
@@ -332,6 +332,7 @@ struct vxlan_dev {#define VXLAN_F_VNIFILTER 0x20000#define VXLAN_F_MDB 0x40000#define VXLAN_F_LOCALBYPASS 0x80000 +#define VXLAN_F_MC_ROUTE 0x100000/* Flags that are used in the receive path. These flags must match in * order for a socket to be shareable
@@ -353,7 +354,9 @@ struct vxlan_dev { VXLAN_F_UDP_ZERO_CSUM6_RX | \ VXLAN_F_COLLECT_METADATA | \ VXLAN_F_VNIFILTER | \ - VXLAN_F_LOCALBYPASS) + VXLAN_F_LOCALBYPASS | \ + VXLAN_F_MC_ROUTE | \ + 0)struct net_device *vxlan_dev_create (struct net *net,const char *name,u8 name_assign_type,struct vxlan_config *conf);
include/uapi/linux/if_link.h +1 -0
@@ -1398,6 +1398,7 @@ enum {IFLA_VXLAN_LOCALBYPASS ,IFLA_VXLAN_LABEL_POLICY ,/* IPv6 flow label policy; ifla_vxlan_label_policy */ IFLA_VXLAN_RESERVED_BITS , +IFLA_VXLAN_MC_ROUTE , __IFLA_VXLAN_MAX };#define IFLA_VXLAN_MAX (__IFLA_VXLAN_MAX - 1)
Signature · 1 line
2.49.0
PATCH v2 11/14 selftests: forwarding: lib: Move smcrouted helpers here
In reply to Petr Machatarouter_multicast.sh has several helpers for work with smcrouted. Extract them to lib.sh so that other selftests can use them as well. Convert the helpers to defer in the process, because that simplifies the interface quite a bit. Therefore have router_multicast.sh invoke defer_scopes_cleanup() in its cleanup() function.
---
Notes:
v2:
- Adjust as per shellcheck citations
---
CC: Shuah Khan <[email protected]>
CC: [email protected]
tools/testing/selftests/net/forwarding/lib.sh | 33 +++++++++++++++++ .../net/forwarding/router_multicast.sh | 35 ++++--------------- 2 files changed, 39 insertions(+), 29 deletions(-)
tools/testing/selftests/net/forwarding/lib.sh +33 -0
@@ -37,6 +37,7 @@ declare -A NETIFS=( : "${TEAMD:=teamd}" : "${MCD:=smcrouted}" : "${MC_CLI:=smcroutectl}" +: "${MCD_TABLE_NAME:=selftests}" # Constants for netdevice bring-up: # Default time in seconds to wait for an interface to come up before giving up
@@ -1757,6 +1758,38 @@ mc_send() msend -g $groups -I $if_name -c 1 > /dev/null 2>&1 } +adf_mcd_start() +{ + local table_name="$MCD_TABLE_NAME" + local smcroutedir + local pid + local i + + check_command "$MCD" || return 1 + check_command "$MC_CLI" || return 1 + + smcroutedir=$(mktemp -d) + defer rm -rf "$smcroutedir" + + for ((i = 1; i <= NUM_NETIFS; ++i)); do + echo "phyint ${NETIFS[p$i]} enable" >> \ + "$smcroutedir/$table_name.conf" + done + + "$MCD" -N -I "$table_name" -f "$smcroutedir/$table_name.conf" \ + -P "$smcroutedir/$table_name.pid" + busywait "$BUSYWAIT_TIMEOUT" test -e "$smcroutedir/$table_name.pid" + pid=$(cat "$smcroutedir/$table_name.pid") + defer kill_process "$pid" +} + +mc_cli() +{ + local table_name="$MCD_TABLE_NAME" + + "$MC_CLI" -I "$table_name" "$@" +} + start_ip_monitor() { local mtype=$1; shift
tools/testing/selftests/net/forwarding/router_multicast.sh +6 -29
@@ -33,10 +33,6 @@ NUM_NETIFS=6 source lib.sh source tc_common.sh -require_command $MCD -require_command $MC_CLI -table_name=selftests - h1_create() { simple_if_init $h1 198.51.100.2/28 2001:db8:1::2/64
@@ -149,25 +145,6 @@ router_destroy() ip link set dev $rp1 down } -start_mcd() -{ - SMCROUTEDIR="$(mktemp -d)" - - for ((i = 1; i <= $NUM_NETIFS; ++i)); do - echo "phyint ${NETIFS[p$i]} enable" >> \ - $SMCROUTEDIR/$table_name.conf - done - - $MCD -N -I $table_name -f $SMCROUTEDIR/$table_name.conf \ - -P $SMCROUTEDIR/$table_name.pid -} - -kill_mcd() -{ - pkill $MCD - rm -rf $SMCROUTEDIR -} - setup_prepare() { h1=${NETIFS[p1]}
@@ -179,7 +156,7 @@ setup_prepare() rp3=${NETIFS[p5]} h3=${NETIFS[p6]} - start_mcd + adf_mcd_start || exit "$EXIT_STATUS" vrf_prepare
@@ -206,7 +183,7 @@ cleanup() vrf_cleanup - kill_mcd + defer_scopes_cleanup } create_mcast_sg()
@@ -214,9 +191,9 @@ create_mcast_sg() local if_name=$1; shift local s_addr=$1; shift local mcast=$1; shift - local dest_ifs=${@} + local dest_ifs=("${@}") - $MC_CLI -I $table_name add $if_name $s_addr $mcast $dest_ifs + mc_cli add "$if_name" "$s_addr" "$mcast" "${dest_ifs[@]}" } delete_mcast_sg()
@@ -224,9 +201,9 @@ delete_mcast_sg() local if_name=$1; shift local s_addr=$1; shift local mcast=$1; shift - local dest_ifs=${@} + local dest_ifs=("${@}") - $MC_CLI -I $table_name remove $if_name $s_addr $mcast $dest_ifs + mc_cli remove "$if_name" "$s_addr" "$mcast" "${dest_ifs[@]}" } mcast_v4()
Signature · 1 line
2.49.0
PATCH v2 12/14 selftests: net: lib: Add ip_link_has_flag()
In reply to Petr MachataAdd a helper to determine whether a given netdevice has a given flag.
Rewrite ip_link_is_up() in terms of the new helper.
---
Notes:
CC: Shuah Khan <[email protected]>
CC: [email protected]
tools/testing/selftests/net/lib.sh | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
tools/testing/selftests/net/lib.sh +9 -3
@@ -547,13 +547,19 @@ ip_link_set_addr() defer ip link set dev "$name" address "$old_addr" } -ip_link_is_up() +ip_link_has_flag() { local name=$1; shift + local flag=$1; shift local state=$(ip -j link show "$name" | - jq -r '(.[].flags[] | select(. == "UP")) // "DOWN"') - [[ $state == "UP" ]] + jq --arg flag "$flag" 'any(.[].flags.[]; . == $flag)') + [[ $state == true ]] +} + +ip_link_is_up() +{ + ip_link_has_flag "$1" UP } ip_link_set_up()
Signature · 1 line
2.49.0
PATCH v2 13/14 selftests: forwarding: adf_mcd_start(): Allow configuring custom interfaces
In reply to Petr MachataTests may wish to add other interfaces to listen on. Notably locally generated traffic uses dummy interfaces. The multicast daemon needs to know about these so that it allows forming rules that involve these interfaces, and so that net.ipv4.conf.X.mc_forwarding is set for the interfaces.
To that end, allow passing in a list of interfaces to configure in addition to all the physical ones.
---
Notes:
v2:
- Adjust as per shellcheck citations
- Retain Nik's R-b, the changes were very minor.
---
CC: Shuah Khan <[email protected]>
CC: [email protected]
tools/testing/selftests/net/forwarding/lib.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+)
tools/testing/selftests/net/forwarding/lib.sh +13 -0
@@ -1760,9 +1760,12 @@ mc_send() adf_mcd_start() { + local ifs=("$@") + local table_name="$MCD_TABLE_NAME" local smcroutedir local pid + local if local i check_command "$MCD" || return 1
@@ -1776,6 +1779,16 @@ adf_mcd_start() "$smcroutedir/$table_name.conf" done + for if in "${ifs[@]}"; do + if ! ip_link_has_flag "$if" MULTICAST; then + ip link set dev "$if" multicast on + defer ip link set dev "$if" multicast off + fi + + echo "phyint $if enable" >> \ + "$smcroutedir/$table_name.conf" + done + "$MCD" -N -I "$table_name" -f "$smcroutedir/$table_name.conf" \ -P "$smcroutedir/$table_name.pid" busywait "$BUSYWAIT_TIMEOUT" test -e "$smcroutedir/$table_name.pid"
Signature · 1 line
2.49.0
PATCH v2 14/14 selftests: forwarding: Add a test for verifying VXLAN MC underlay
In reply to Petr MachataAdd tests for MC-routing underlay VXLAN traffic.
---
Notes:
v2:
- Adjust as per shellcheck citations
---
CC: Shuah Khan <[email protected]>
CC: [email protected]
.../testing/selftests/net/forwarding/Makefile | 1 + .../net/forwarding/vxlan_bridge_1q_mc_ul.sh | 769 ++++++++++++++++++ 2 files changed, 770 insertions(+) create mode 100755 tools/testing/selftests/net/forwarding/vxlan_bridge_1q_mc_ul.sh
tools/testing/selftests/net/forwarding/Makefile +1 -0
@@ -102,6 +102,7 @@ TEST_PROGS = bridge_fdb_learning_limit.sh \ vxlan_bridge_1d_port_8472.sh \ vxlan_bridge_1d.sh \ vxlan_bridge_1q_ipv6.sh \ + vxlan_bridge_1q_mc_ul.sh \ vxlan_bridge_1q_port_8472_ipv6.sh \ vxlan_bridge_1q_port_8472.sh \ vxlan_bridge_1q.sh \
tools/testing/selftests/net/forwarding/vxlan_bridge_1q_mc_ul.sh new file +769 -0
769 lines not on this page · raw message
Signature · 1 line
2.49.0
Jakub Kicinski
maintainerRe: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay
Quoted 8 lines · On Thu, 12 Jun 2025 22:10:48 +0200 Petr Machata wrote:
Add tests for MC-routing underlay VXLAN traffic.
Signed-off-by Petr Machata <[email protected]>---
Notes:
v2:
- Adjust as per shellcheck citations
Noob question - would we also be able to squash the unreachable code warnings if we declared ALL_TESTS as an array instead of a string? IDK if there's any trick we could use to make shellcheck stop complaining. Not blocking the series, obviously.
CC Matthieu, I presume you may have already investigated this :)
Re: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay
Hi Jakub, Petr,
Quoted 10 lines · Jakub Kicinski
Quoted 8 lines · On Thu, 12 Jun 2025 22:10:48 +0200 Petr Machata wrote:
Add tests for MC-routing underlay VXLAN traffic.
Signed-off-by Petr Machata <[email protected]>---
Notes:
v2:
- Adjust as per shellcheck citations
Quoted 6 lines
Noob question - would we also be able to squash the unreachable code warnings if we declared
ALL_TESTSas an array instead of a string? IDK if there's any trick we could use to make shellcheck stop complaining. Not blocking the series, obviously.CC Matthieu, I presume you may have already investigated this :)
Thank you for the Cc. Yes indeed, I already had this case.
I don't think declaring ALL_TESTS as an array would help for this case -- even if it looks clearer than a long string -- because I think shellcheck will simply check if all the different functions are called directly. As mentioned in Shellcheck wiki [1]: "ShellCheck may incorrectly believe that code is unreachable if it's invoked by variable name or in a trap. In such a case, please Ignore the message".
That what we did with MPTCP, see the top of the mptcp_join.sh file for example [2], where we have:
Quoted 3 lines
# ShellCheck incorrectly believes that most of the code here is unreachable
# because it's invoked by variable name, see how the "tests" array is used #shellcheck disable=SC2317
If you add this at the top of your new file, followed by an empty line, shellcheck will ignore this issue for the whole file.
Note: regarding the other issue you have:
Quoted 3 lines
In
vxlan_bridge_1q_mc_ul.shline 766:setup_wait
^--------^ SC2119 (info): Usesetup_wait"$@" if function's $1 should mean script's $1.
I guess you can also ignore it, or use "" as argument. If you ignore it -- which looks cleaner -- I think it is always good to add a comment, e.g.
Quoted 2 lines
# shellcheck disable=SC2119 # arguments are optional, not needed here. setup_wait
[1] https://www.shellcheck.net/wiki/SC2317
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/tools/testing/selftests/net/mptcp/mptcp_join.sh#n9
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.
Re: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay
Quoted 32 lines · Matthieu Baerts
Hi Jakub, Petr,
Quoted 10 lines · Jakub Kicinski
Quoted 8 lines · On Thu, 12 Jun 2025 22:10:48 +0200 Petr Machata wrote:
Add tests for MC-routing underlay VXLAN traffic.
Signed-off-by Petr Machata <[email protected]>---
Notes:
v2:
- Adjust as per shellcheck citationsQuoted 6 lines
Noob question - would we also be able to squash the unreachable code warnings if we declared
ALL_TESTSas an array instead of a string? IDK if there's any trick we could use to make shellcheck stop complaining. Not blocking the series, obviously.CC Matthieu, I presume you may have already investigated this :)
Thank you for the Cc. Yes indeed, I already had this case.
I don't think declaring
ALL_TESTSas an array would help for this case -- even if it looks clearer than a long string -- because I think shellcheck will simply check if all the different functions are called directly. As mentioned in Shellcheck wiki [1]: "ShellCheck may incorrectly believe that code is unreachable if it's invoked by variable name or in a trap. In such a case, please Ignore the message".That what we did with MPTCP, see the top of the
mptcp_join.shfile for example [2], where we have:
Quoted 6 lines
Quoted 3 lines
# ShellCheck incorrectly believes that most of the code here is unreachable
# because it's invoked by variable name, see how the "tests" array is used #shellcheck disable=SC2317If you add this at the top of your new file, followed by an empty line, shellcheck will ignore this issue for the whole file.
The ALL_TESTS issue is not the end of it either. We use helpers that call stuff indirectly all over the place. defer, in_ns... It would make sense to me to just disable SC2317 in NIPA runs. Or maybe even put it in net/forwarding/.shellcheckrc. Pretty much all those tests are written in a style that will hit these issues.
Quoted 11 lines
Note: regarding the other issue you have:
Quoted 3 lines
In
vxlan_bridge_1q_mc_ul.shline 766:setup_wait
^--------^ SC2119 (info): Usesetup_wait"$@" if function's $1 should mean script's $1.I guess you can also ignore it, or use "" as argument. If you ignore it -- which looks cleaner -- I think it is always good to add a comment, e.g.
Quoted 2 lines
# shellcheck disable=SC2119 # arguments are optional, not needed here. setup_wait
I'll just pass "$NUM_NETIFS" to get rid of the warning. I really don't like these shellcheck annotations.
I'll send a patch that changes the calling convention so that SC doesn't get triggered, because I really don't want every new selftest to have to deal with this.
Re: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay
Hi Petr,
Quoted 39 lines · Petr Machata
Quoted 36 lines · Matthieu Baerts
Hi Jakub, Petr,
Quoted 10 lines · Jakub Kicinski
Quoted 8 lines · On Thu, 12 Jun 2025 22:10:48 +0200 Petr Machata wrote:
Add tests for MC-routing underlay VXLAN traffic.
Signed-off-by Petr Machata <[email protected]>---
Notes:
v2:
- Adjust as per shellcheck citationsQuoted 6 lines
Noob question - would we also be able to squash the unreachable code warnings if we declared
ALL_TESTSas an array instead of a string? IDK if there's any trick we could use to make shellcheck stop complaining. Not blocking the series, obviously.CC Matthieu, I presume you may have already investigated this :)
Thank you for the Cc. Yes indeed, I already had this case.
I don't think declaring
ALL_TESTSas an array would help for this case -- even if it looks clearer than a long string -- because I think shellcheck will simply check if all the different functions are called directly. As mentioned in Shellcheck wiki [1]: "ShellCheck may incorrectly believe that code is unreachable if it's invoked by variable name or in a trap. In such a case, please Ignore the message".That what we did with MPTCP, see the top of the
mptcp_join.shfile for example [2], where we have:Quoted 3 lines
# ShellCheck incorrectly believes that most of the code here is unreachable
# because it's invoked by variable name, see how the "tests" array is used #shellcheck disable=SC2317
Quoted 8 lines
Quoted 2 lines
If you add this at the top of your new file, followed by an empty line, shellcheck will ignore this issue for the whole file.
The
ALL_TESTSissue is not the end of it either. We use helpers that call stuff indirectly all over the place. defer,in_ns... It would make sense to me to just disable SC2317 in NIPA runs. Or maybe even put it innet/forwarding/.shellcheckrc. Pretty much all those tests are written in a style that will hit these issues.
In this case, I think it would be better to add this .shellcheckrc file in net/forwarding. If you modify NIPA, I don't think people will know what is allowed or not, or what command line to use, no?
Note that NIPA's shellcheck reports are currently ignoring all "info" and "style" severities -- so including SC2317 -- except for new files or the ones that were previously shellcheck compliant.
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.
Jakub Kicinski
maintainerRe: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay
Quoted 9 lines · On Mon, 16 Jun 2025 18:18:18 +0200 Matthieu Baerts wrote:
Quoted 5 lines
The
ALL_TESTSissue is not the end of it either. We use helpers that call stuff indirectly all over the place. defer,in_ns... It would make sense to me to just disable SC2317 in NIPA runs. Or maybe even put it innet/forwarding/.shellcheckrc. Pretty much all those tests are written in a style that will hit these issues.In this case, I think it would be better to add this .shellcheckrc file in net/forwarding. If you modify NIPA, I don't think people will know what is allowed or not, or what command line to use, no?
Let's do both? I disabled it to NIPA over the weekend.
Re: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay
In reply to Matthieu BaertsQuoted 22 lines · Matthieu Baerts
Hi Petr,
Quoted 10 lines · Petr Machata
Quoted 7 lines · Matthieu Baerts
That what we did with MPTCP, see the top of the
mptcp_join.shfile for example [2], where we have:Quoted 3 lines
# ShellCheck incorrectly believes that most of the code here is unreachable
# because it's invoked by variable name, see how the "tests" array is used #shellcheck disable=SC2317Quoted 8 lines
Quoted 2 lines
If you add this at the top of your new file, followed by an empty line, shellcheck will ignore this issue for the whole file.
The
ALL_TESTSissue is not the end of it either. We use helpers that call stuff indirectly all over the place. defer,in_ns... It would make sense to me to just disable SC2317 in NIPA runs. Or maybe even put it innet/forwarding/.shellcheckrc. Pretty much all those tests are written in a style that will hit these issues.
Quoted 3 lines
In this case, I think it would be better to add this .shellcheckrc file in net/forwarding. If you modify NIPA, I don't think people will know what is allowed or not, or what command line to use, no?
Good point. The question then is whether to put it to forwarding/ or directly net/, which is has seen more use of lib.sh and therefore the same sort of coding style. I'll experiment with it and should be able to send it later in the week. I don't want to add it to the MC patchset.
Quoted 3 lines
Note that NIPA's shellcheck reports are currently ignoring all "info" and "style" severities -- so including SC2317 -- except for new files or the ones that were previously shellcheck compliant.
Yeah, I know, but the result is still very noisy, if you want to verify it prior to sending the patchset. It's a bit annoying to have to scroll through the report trying to find relevant stuff. I could add .shellcheckrc in my own clone, but everybody is going to hit these.
Re: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay
Quoted 25 lines · Petr Machata
Quoted 22 lines · Matthieu Baerts
Hi Petr,
Quoted 18 lines · Petr Machata
Quoted 9 lines · Matthieu Baerts
That what we did with MPTCP, see the top of the
mptcp_join.shfile for example [2], where we have:Quoted 3 lines
# ShellCheck incorrectly believes that most of the code here is unreachable
# because it's invoked by variable name, see how the "tests" array is used #shellcheck disable=SC2317If you add this at the top of your new file, followed by an empty line, shellcheck will ignore this issue for the whole file.
The
ALL_TESTSissue is not the end of it either. We use helpers that call stuff indirectly all over the place. defer,in_ns... It would make sense to me to just disable SC2317 in NIPA runs. Or maybe even put it innet/forwarding/.shellcheckrc. Pretty much all those tests are written in a style that will hit these issues.
Quoted 8 lines
Quoted 3 lines
In this case, I think it would be better to add this .shellcheckrc file in net/forwarding. If you modify NIPA, I don't think people will know what is allowed or not, or what command line to use, no?
Good point. The question then is whether to put it to forwarding/ or directly
net/, which is has seen more use oflib.shand therefore the same sort of coding style. I'll experiment with it and should be able to send it later in the week. I don't want to add it to the MC patchset.
Since Jakub disabled SC2317 in NIPA [1], then I guess we can put this .shellcheckrc file in net/, no?
Quoted 8 lines
Quoted 3 lines
Note that NIPA's shellcheck reports are currently ignoring all "info" and "style" severities -- so including SC2317 -- except for new files or the ones that were previously shellcheck compliant.
Yeah, I know, but the result is still very noisy, if you want to verify it prior to sending the patchset. It's a bit annoying to have to scroll through the report trying to find relevant stuff. I could add .shellcheckrc in my own clone, but everybody is going to hit these.
When Shellcheck got introduced, there were some discussions about SC2317 and SC2086 [2]. I don't think they are useless -- maybe a function is not used by mistake, maybe double quotes are really needed for some cases, etc. -- but I agree with you: they create a lot of noise.
[1] https://github.com/linux-netdev/nipa/commit/23b74dd
[2] https://github.com/linux-netdev/nipa/pull/51#issuecomment-2939556291
Cheers,
Matt
Signature · 1 line
Sponsored by the NGI0 Core fund.
Jakub Kicinski
maintainerRe: [PATCH net-next v2 14/14] selftests: forwarding: Add a test for verifying VXLAN MC underlay
Quoted 7 lines · On Mon, 16 Jun 2025 22:40:49 +0200 Matthieu Baerts wrote:
Quoted 4 lines
Good point. The question then is whether to put it to forwarding/ or directly
net/, which is has seen more use oflib.shand therefore the same sort of coding style. I'll experiment with it and should be able to send it later in the week. I don't want to add it to the MC patchset.Since Jakub disabled SC2317 in NIPA [1], then I guess we can put this .shellcheckrc file in
net/, no?
And drivers/net while at it, if you don't mind.
Jakub Kicinski
maintainerQuoted 6 lines · On Thu, 12 Jun 2025 22:10:34 +0200 Petr Machata wrote:
Multicast routing is today handled in the input path. Locally generated MC packets don't hit the IPMR code. Thus if a VXLAN remote address is multicast, the driver needs to set an OIF during route lookup. In practice that means that MC routing configuration needs to be kept in sync with the VXLAN FDB and MDB. Ideally, the VXLAN packets would be routed by the MC routing code instead.
Crazy, the leaks are back in NIPA. 100% reproducible it seems, the patches have been in 3 branches and every time the first run and the retry is reporting leaks. I'll try to dig into this a bit, but I'm gonna set the patches as Deferred in PW in the meantime to avoid blocking the queue.