On 11/12/2020 11:26, Horatiu Vultur wrote: > This patch tries to add vlan support to IGMP queries. > It extends the function 'br_ip4_multicast_alloc_query' to add > also a vlan tag if vlan is enabled. Therefore the bridge will send > queries for each vlan the ports are in. > > There are few other places that needs to be updated to be fully > functional. But I am curious if this is the way to go forward or is > there a different way of implementing this? > > Signed-off-by: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx> > --- > net/bridge/br_multicast.c | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) > Hi Horatiu, We've discussed this with other people on netdev before, the way forward is to implement it as a per-vlan option and then have a per-vlan querier. Which would also make the change much bigger and more complex. In general some of the multicast options need to be replicated for vlans to get proper per-vlan multicast control and operation, but that would require to change a lot of logic around the whole bridge (fast-path included, where it'd be most sensitive). The good news is that these days we have per-vlan options support and so only the actual per-vlan multicast implementation is left to be done. I have this on my TODO list, unfortunately that list gets longer and longer, so I'd be happy to review patches if someone decides to do it sooner. :) Sorry, I couldn't find the previous discussion, it was a few years back. Cheers, Nik > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c > index 484820c223a3..4c2db8a9efe0 100644 > --- a/net/bridge/br_multicast.c > +++ b/net/bridge/br_multicast.c > @@ -688,7 +688,8 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br, > __be32 ip_dst, __be32 group, > bool with_srcs, bool over_lmqt, > u8 sflag, u8 *igmp_type, > - bool *need_rexmit) > + bool *need_rexmit, > + __u16 vid) > { > struct net_bridge_port *p = pg ? pg->key.port : NULL; > struct net_bridge_group_src *ent; > @@ -724,6 +725,9 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br, > } > > pkt_size = sizeof(*eth) + sizeof(*iph) + 4 + igmp_hdr_size; > + if (br_vlan_enabled(br->dev) && vid != 0) > + pkt_size += 4; > + > if ((p && pkt_size > p->dev->mtu) || > pkt_size > br->dev->mtu) > return NULL; > @@ -732,6 +736,9 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br, > if (!skb) > goto out; > > + if (br_vlan_enabled(br->dev) && vid != 0) > + __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid); > + > skb->protocol = htons(ETH_P_IP); > > skb_reset_mac_header(skb); > @@ -1008,7 +1015,8 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br, > ip4_dst, group->dst.ip4, > with_srcs, over_lmqt, > sflag, igmp_type, > - need_rexmit); > + need_rexmit, > + group->vid); > #if IS_ENABLED(CONFIG_IPV6) > case htons(ETH_P_IPV6): { > struct in6_addr ip6_dst; > @@ -1477,6 +1485,8 @@ static void br_multicast_send_query(struct net_bridge *br, > struct bridge_mcast_own_query *own_query) > { > struct bridge_mcast_other_query *other_query = NULL; > + struct net_bridge_vlan_group *vg; > + struct net_bridge_vlan *v; > struct br_ip br_group; > unsigned long time; > > @@ -1485,7 +1495,7 @@ static void br_multicast_send_query(struct net_bridge *br, > !br_opt_get(br, BROPT_MULTICAST_QUERIER)) > return; > > - memset(&br_group.dst, 0, sizeof(br_group.dst)); > + memset(&br_group, 0, sizeof(br_group)); > > if (port ? (own_query == &port->ip4_own_query) : > (own_query == &br->ip4_own_query)) { > @@ -1501,8 +1511,19 @@ static void br_multicast_send_query(struct net_bridge *br, > if (!other_query || timer_pending(&other_query->timer)) > return; > > - __br_multicast_send_query(br, port, NULL, NULL, &br_group, false, 0, > - NULL); > + if (br_vlan_enabled(br->dev) && port) { > + vg = nbp_vlan_group(port); > + > + list_for_each_entry(v, &vg->vlan_list, vlist) { > + br_group.vid = v->vid == vg->pvid ? 0 : v->vid; > + > + __br_multicast_send_query(br, port, NULL, NULL, > + &br_group, false, 0, NULL); > + } > + } else { > + __br_multicast_send_query(br, port, NULL, NULL, &br_group, > + false, 0, NULL); > + } > > time = jiffies; > time += own_query->startup_sent < br->multicast_startup_query_count ? >