On 10/24/24 12:30, MD Danish Anwar wrote:
From: Murali Karicheri m-karicheri2@ti.com
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@ti.com Signed-off-by: MD Danish Anwar danishanwar@ti.com
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