On 10/24/24 12:30, MD Danish Anwar wrote: > From: Murali Karicheri <m-karicheri2@xxxxxx> > > This patch adds support for VLAN ctag based filtering at slave devices. > The slave ethernet device may be capable of filtering ethernet packets > based on VLAN ID. This requires that when the VLAN interface is created > over an HSR/PRP interface, it passes the VID information to the > associated slave ethernet devices so that it updates the hardware > filters to filter ethernet frames based on VID. This patch adds the > required functions to propagate the vid information to the slave > devices. > > Signed-off-by: Murali Karicheri <m-karicheri2@xxxxxx> > Signed-off-by: MD Danish Anwar <danishanwar@xxxxxx> > --- > net/hsr/hsr_device.c | 71 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 70 insertions(+), 1 deletion(-) > > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c > index 0ca47ebb01d3..ff586bdc2bde 100644 > --- a/net/hsr/hsr_device.c > +++ b/net/hsr/hsr_device.c > @@ -515,6 +515,68 @@ static void hsr_change_rx_flags(struct net_device *dev, int change) > } > } > > +static int hsr_ndo_vlan_rx_add_vid(struct net_device *dev, > + __be16 proto, u16 vid) > +{ > + struct hsr_port *port; > + struct hsr_priv *hsr; > + int ret = 0; > + > + hsr = netdev_priv(dev); > + > + hsr_for_each_port(hsr, port) { > + if (port->type == HSR_PT_MASTER) > + continue; If the desired behavior is to ignore INTERLINK port, I think you should explicitly skip them here, otherwise you will end-up in a nondeterministic state. > + ret = vlan_vid_add(port->dev, proto, vid); > + switch (port->type) { > + case HSR_PT_SLAVE_A: > + if (ret) { > + netdev_err(dev, "add vid failed for Slave-A\n"); > + return ret; > + } > + break; > + > + case HSR_PT_SLAVE_B: > + if (ret) { > + /* clean up Slave-A */ > + netdev_err(dev, "add vid failed for Slave-B\n"); > + vlan_vid_del(port->dev, proto, vid); This code relies on a specific port_list order - which is actually respected at list creation time. Still such assumption looks fragile and may lead to long term bugs. I think would be better to refactor the above loop handling arbitrary HSR_PT_SLAVE_A, HSR_PT_SLAVE_B order. Guestimate is that the complexity will not increase measurably. > + return ret; > + } > + break; > + default: > + break; > + } > + } > + > + return 0; > +} > + > +static int hsr_ndo_vlan_rx_kill_vid(struct net_device *dev, > + __be16 proto, u16 vid) > +{ > + struct hsr_port *port; > + struct hsr_priv *hsr; > + > + hsr = netdev_priv(dev); > + > + hsr_for_each_port(hsr, port) { > + if (port->type == HSR_PT_MASTER) > + continue; I think it would be more consistent just removing the above statement... > + switch (port->type) { > + case HSR_PT_SLAVE_A: > + case HSR_PT_SLAVE_B: > + vlan_vid_del(port->dev, proto, vid); > + break; > + default:> + break; ... MASTER and INTERLINK port will be ignored anyway. > + } > + } > + > + return 0; > +} > + > static const struct net_device_ops hsr_device_ops = { > .ndo_change_mtu = hsr_dev_change_mtu, > .ndo_open = hsr_dev_open, Cheers, Paolo