2023-06-12 10:14:01

by Ravi Gunasekaran

[permalink] [raw]
Subject: [PATCH] net: hsr: Disable promiscuous mode in offload mode

When port-to-port forwarding for interfaces in HSR node is enabled,
disable promiscuous mode since L2 frame forward happens at the
offloaded hardware.

Signed-off-by: Ravi Gunasekaran <[email protected]>
---
net/hsr/hsr_device.c | 5 +++++
net/hsr/hsr_main.h | 1 +
net/hsr/hsr_slave.c | 15 +++++++++++----
3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 5a236aae2366..306f942c3b28 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -531,6 +531,11 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
if (res)
goto err_add_master;

+ /* HSR forwarding offload supported in lower device? */
+ if ((slave[0]->features & NETIF_F_HW_HSR_FWD) &&
+ (slave[1]->features & NETIF_F_HW_HSR_FWD))
+ hsr->fwd_offloaded = true;
+
res = register_netdevice(hsr_dev);
if (res)
goto err_unregister;
diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
index 5584c80a5c79..0225fabbe6d1 100644
--- a/net/hsr/hsr_main.h
+++ b/net/hsr/hsr_main.h
@@ -195,6 +195,7 @@ struct hsr_priv {
struct hsr_self_node __rcu *self_node; /* MACs of slaves */
struct timer_list announce_timer; /* Supervision frame dispatch */
struct timer_list prune_timer;
+ unsigned int fwd_offloaded : 1; /* Forwarding offloaded to HW */
int announce_count;
u16 sequence_nr;
u16 sup_sequence_nr; /* For HSRv1 separate seq_nr for supervision */
diff --git a/net/hsr/hsr_slave.c b/net/hsr/hsr_slave.c
index b70e6bbf6021..e5742f2a2d52 100644
--- a/net/hsr/hsr_slave.c
+++ b/net/hsr/hsr_slave.c
@@ -131,9 +131,14 @@ static int hsr_portdev_setup(struct hsr_priv *hsr, struct net_device *dev,
struct hsr_port *master;
int res;

- res = dev_set_promiscuity(dev, 1);
- if (res)
- return res;
+ /* Don't use promiscuous mode for offload since L2 frame forward
+ * happens at the offloaded hardware.
+ */
+ if (!port->hsr->fwd_offloaded) {
+ res = dev_set_promiscuity(dev, 1);
+ if (res)
+ return res;
+ }

master = hsr_port_get_hsr(hsr, HSR_PT_MASTER);
hsr_dev = master->dev;
@@ -152,7 +157,9 @@ static int hsr_portdev_setup(struct hsr_priv *hsr, struct net_device *dev,
fail_rx_handler:
netdev_upper_dev_unlink(dev, hsr_dev);
fail_upper_dev_link:
- dev_set_promiscuity(dev, -1);
+ if (!port->hsr->fwd_offloaded)
+ dev_set_promiscuity(dev, -1);
+
return res;
}

--
2.17.1



2023-06-14 08:12:38

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH] net: hsr: Disable promiscuous mode in offload mode

On Mon, Jun 12, 2023 at 03:09:33PM +0530, Ravi Gunasekaran wrote:
> When port-to-port forwarding for interfaces in HSR node is enabled,
> disable promiscuous mode since L2 frame forward happens at the
> offloaded hardware.
>
> Signed-off-by: Ravi Gunasekaran <[email protected]>

Reviewed-by: Simon Horman <[email protected]>


2023-06-14 09:56:00

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH] net: hsr: Disable promiscuous mode in offload mode

On Mon, 2023-06-12 at 15:09 +0530, Ravi Gunasekaran wrote:
> When port-to-port forwarding for interfaces in HSR node is enabled,
> disable promiscuous mode since L2 frame forward happens at the
> offloaded hardware.
>
> Signed-off-by: Ravi Gunasekaran <[email protected]>
> ---
> net/hsr/hsr_device.c | 5 +++++
> net/hsr/hsr_main.h | 1 +
> net/hsr/hsr_slave.c | 15 +++++++++++----
> 3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index 5a236aae2366..306f942c3b28 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -531,6 +531,11 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
> if (res)
> goto err_add_master;
>
> + /* HSR forwarding offload supported in lower device? */
> + if ((slave[0]->features & NETIF_F_HW_HSR_FWD) &&
> + (slave[1]->features & NETIF_F_HW_HSR_FWD))
> + hsr->fwd_offloaded = true;
> +
> res = register_netdevice(hsr_dev);
> if (res)
> goto err_unregister;
> diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
> index 5584c80a5c79..0225fabbe6d1 100644
> --- a/net/hsr/hsr_main.h
> +++ b/net/hsr/hsr_main.h
> @@ -195,6 +195,7 @@ struct hsr_priv {
> struct hsr_self_node __rcu *self_node; /* MACs of slaves */
> struct timer_list announce_timer; /* Supervision frame dispatch */
> struct timer_list prune_timer;
> + unsigned int fwd_offloaded : 1; /* Forwarding offloaded to HW */

Please use plain 'bool' instead.

Also there is an hole in 'struct hsr_priv' just after 'net_id', you
could consider moving this new field there.


Thanks!

Paolo


2023-06-14 10:03:18

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH] net: hsr: Disable promiscuous mode in offload mode

On Wed, 2023-06-14 at 11:42 +0200, Paolo Abeni wrote:
> On Mon, 2023-06-12 at 15:09 +0530, Ravi Gunasekaran wrote:
> > When port-to-port forwarding for interfaces in HSR node is enabled,
> > disable promiscuous mode since L2 frame forward happens at the
> > offloaded hardware.
> >
> > Signed-off-by: Ravi Gunasekaran <[email protected]>
> > ---
> > net/hsr/hsr_device.c | 5 +++++
> > net/hsr/hsr_main.h | 1 +
> > net/hsr/hsr_slave.c | 15 +++++++++++----
> > 3 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> > index 5a236aae2366..306f942c3b28 100644
> > --- a/net/hsr/hsr_device.c
> > +++ b/net/hsr/hsr_device.c
> > @@ -531,6 +531,11 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
> > if (res)
> > goto err_add_master;
> >
> > + /* HSR forwarding offload supported in lower device? */
> > + if ((slave[0]->features & NETIF_F_HW_HSR_FWD) &&
> > + (slave[1]->features & NETIF_F_HW_HSR_FWD))
> > + hsr->fwd_offloaded = true;
> > +
> > res = register_netdevice(hsr_dev);
> > if (res)
> > goto err_unregister;
> > diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
> > index 5584c80a5c79..0225fabbe6d1 100644
> > --- a/net/hsr/hsr_main.h
> > +++ b/net/hsr/hsr_main.h
> > @@ -195,6 +195,7 @@ struct hsr_priv {
> > struct hsr_self_node __rcu *self_node; /* MACs of slaves */
> > struct timer_list announce_timer; /* Supervision frame dispatch */
> > struct timer_list prune_timer;
> > + unsigned int fwd_offloaded : 1; /* Forwarding offloaded to HW */
>
> Please use plain 'bool' instead.
>
> Also there is an hole in 'struct hsr_priv' just after 'net_id', you
> could consider moving this new field there.

Oops, I almost forgot! Please include the target tree (net-next in this
case) in the subj prefix on your next submission.

Thanks,

Paolo


2023-06-14 10:23:55

by Ravi Gunasekaran

[permalink] [raw]
Subject: Re: [PATCH] net: hsr: Disable promiscuous mode in offload mode



On 6/14/23 3:14 PM, Paolo Abeni wrote:
> On Wed, 2023-06-14 at 11:42 +0200, Paolo Abeni wrote:
>> On Mon, 2023-06-12 at 15:09 +0530, Ravi Gunasekaran wrote:
>>> When port-to-port forwarding for interfaces in HSR node is enabled,
>>> disable promiscuous mode since L2 frame forward happens at the
>>> offloaded hardware.
>>>
>>> Signed-off-by: Ravi Gunasekaran <[email protected]>
>>> ---
>>> net/hsr/hsr_device.c | 5 +++++
>>> net/hsr/hsr_main.h | 1 +
>>> net/hsr/hsr_slave.c | 15 +++++++++++----
>>> 3 files changed, 17 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
>>> index 5a236aae2366..306f942c3b28 100644
>>> --- a/net/hsr/hsr_device.c
>>> +++ b/net/hsr/hsr_device.c
>>> @@ -531,6 +531,11 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
>>> if (res)
>>> goto err_add_master;
>>>
>>> + /* HSR forwarding offload supported in lower device? */
>>> + if ((slave[0]->features & NETIF_F_HW_HSR_FWD) &&
>>> + (slave[1]->features & NETIF_F_HW_HSR_FWD))
>>> + hsr->fwd_offloaded = true;
>>> +
>>> res = register_netdevice(hsr_dev);
>>> if (res)
>>> goto err_unregister;
>>> diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
>>> index 5584c80a5c79..0225fabbe6d1 100644
>>> --- a/net/hsr/hsr_main.h
>>> +++ b/net/hsr/hsr_main.h
>>> @@ -195,6 +195,7 @@ struct hsr_priv {
>>> struct hsr_self_node __rcu *self_node; /* MACs of slaves */
>>> struct timer_list announce_timer; /* Supervision frame dispatch */
>>> struct timer_list prune_timer;
>>> + unsigned int fwd_offloaded : 1; /* Forwarding offloaded to HW */
>>
>> Please use plain 'bool' instead.
>>
>> Also there is an hole in 'struct hsr_priv' just after 'net_id', you
>> could consider moving this new field there.
>
> Oops, I almost forgot! Please include the target tree (net-next in this
> case) in the subj prefix on your next submission.
>

I will take care of this from next submission onwards.

> Thanks,
>
> Paolo
>

--
Regards,
Ravi

2023-06-14 10:31:21

by Ravi Gunasekaran

[permalink] [raw]
Subject: Re: [PATCH] net: hsr: Disable promiscuous mode in offload mode



On 6/14/23 3:12 PM, Paolo Abeni wrote:
> On Mon, 2023-06-12 at 15:09 +0530, Ravi Gunasekaran wrote:
>> When port-to-port forwarding for interfaces in HSR node is enabled,
>> disable promiscuous mode since L2 frame forward happens at the
>> offloaded hardware.
>>
>> Signed-off-by: Ravi Gunasekaran <[email protected]>
>> ---
>> net/hsr/hsr_device.c | 5 +++++
>> net/hsr/hsr_main.h | 1 +
>> net/hsr/hsr_slave.c | 15 +++++++++++----
>> 3 files changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
>> index 5a236aae2366..306f942c3b28 100644
>> --- a/net/hsr/hsr_device.c
>> +++ b/net/hsr/hsr_device.c
>> @@ -531,6 +531,11 @@ int hsr_dev_finalize(struct net_device *hsr_dev, struct net_device *slave[2],
>> if (res)
>> goto err_add_master;
>>
>> + /* HSR forwarding offload supported in lower device? */
>> + if ((slave[0]->features & NETIF_F_HW_HSR_FWD) &&
>> + (slave[1]->features & NETIF_F_HW_HSR_FWD))
>> + hsr->fwd_offloaded = true;
>> +
>> res = register_netdevice(hsr_dev);
>> if (res)
>> goto err_unregister;
>> diff --git a/net/hsr/hsr_main.h b/net/hsr/hsr_main.h
>> index 5584c80a5c79..0225fabbe6d1 100644
>> --- a/net/hsr/hsr_main.h
>> +++ b/net/hsr/hsr_main.h
>> @@ -195,6 +195,7 @@ struct hsr_priv {
>> struct hsr_self_node __rcu *self_node; /* MACs of slaves */
>> struct timer_list announce_timer; /* Supervision frame dispatch */
>> struct timer_list prune_timer;
>> + unsigned int fwd_offloaded : 1; /* Forwarding offloaded to HW */
>
> Please use plain 'bool' instead.
>
> Also there is an hole in 'struct hsr_priv' just after 'net_id', you
> could consider moving this new field there.
>

Sure. I will use "bool" and insert it after "net_id" and send out v2

>
> Thanks!
>
> Paolo
>

--
Regards,
Ravi