* [PATCH] net: ethernet: ti: ale: ensure vlan/mdb deleted when no members
@ 2019-11-27 15:59 Grygorii Strashko
2019-11-28 8:21 ` Ilias Apalodimas
0 siblings, 1 reply; 4+ messages in thread
From: Grygorii Strashko @ 2019-11-27 15:59 UTC (permalink / raw)
To: netdev, David S . Miller
Cc: Ilias Apalodimas, Andrew Lunn, Sekhar Nori, Ivan Khoronzhuk,
linux-kernel, linux-omap, Grygorii Strashko
The recently updated ALE APIs cpsw_ale_del_mcast() and
cpsw_ale_del_vlan_modify() have an issue and will not delete ALE entry even
if VLAN/mcast group has no more members. Hence fix it here and delete ALE
entry if !port_mask.
The issue affected only new cpsw switchdev driver.
Fixes: e85c14370783 ("net: ethernet: ti: ale: modify vlan/mdb api for switchdev")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
drivers/net/ethernet/ti/cpsw_ale.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c
index 929f3d3354e3..a5179ecfea05 100644
--- a/drivers/net/ethernet/ti/cpsw_ale.c
+++ b/drivers/net/ethernet/ti/cpsw_ale.c
@@ -396,12 +396,14 @@ int cpsw_ale_del_mcast(struct cpsw_ale *ale, const u8 *addr, int port_mask,
if (port_mask) {
mcast_members = cpsw_ale_get_port_mask(ale_entry,
ale->port_mask_bits);
- mcast_members &= ~port_mask;
- cpsw_ale_set_port_mask(ale_entry, mcast_members,
+ port_mask = mcast_members & ~port_mask;
+ }
+
+ if (port_mask)
+ cpsw_ale_set_port_mask(ale_entry, port_mask,
ale->port_mask_bits);
- } else {
+ else
cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE);
- }
cpsw_ale_write(ale, idx, ale_entry);
return 0;
@@ -478,6 +480,10 @@ static void cpsw_ale_del_vlan_modify(struct cpsw_ale *ale, u32 *ale_entry,
members = cpsw_ale_get_vlan_member_list(ale_entry,
ale->vlan_field_bits);
members &= ~port_mask;
+ if (!members) {
+ cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE);
+ return;
+ }
untag = cpsw_ale_get_vlan_untag_force(ale_entry,
ale->vlan_field_bits);
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] net: ethernet: ti: ale: ensure vlan/mdb deleted when no members
2019-11-27 15:59 [PATCH] net: ethernet: ti: ale: ensure vlan/mdb deleted when no members Grygorii Strashko
@ 2019-11-28 8:21 ` Ilias Apalodimas
2019-11-28 9:38 ` Ilias Apalodimas
0 siblings, 1 reply; 4+ messages in thread
From: Ilias Apalodimas @ 2019-11-28 8:21 UTC (permalink / raw)
To: Grygorii Strashko
Cc: netdev, David S . Miller, Andrew Lunn, Sekhar Nori,
Ivan Khoronzhuk, linux-kernel, linux-omap
On Wed, Nov 27, 2019 at 05:59:05PM +0200, Grygorii Strashko wrote:
> The recently updated ALE APIs cpsw_ale_del_mcast() and
> cpsw_ale_del_vlan_modify() have an issue and will not delete ALE entry even
> if VLAN/mcast group has no more members. Hence fix it here and delete ALE
> entry if !port_mask.
>
> The issue affected only new cpsw switchdev driver.
>
> Fixes: e85c14370783 ("net: ethernet: ti: ale: modify vlan/mdb api for switchdev")
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> drivers/net/ethernet/ti/cpsw_ale.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c
> index 929f3d3354e3..a5179ecfea05 100644
> --- a/drivers/net/ethernet/ti/cpsw_ale.c
> +++ b/drivers/net/ethernet/ti/cpsw_ale.c
> @@ -396,12 +396,14 @@ int cpsw_ale_del_mcast(struct cpsw_ale *ale, const u8 *addr, int port_mask,
> if (port_mask) {
> mcast_members = cpsw_ale_get_port_mask(ale_entry,
> ale->port_mask_bits);
> - mcast_members &= ~port_mask;
> - cpsw_ale_set_port_mask(ale_entry, mcast_members,
> + port_mask = mcast_members & ~port_mask;
> + }
> +
> + if (port_mask)
> + cpsw_ale_set_port_mask(ale_entry, port_mask,
> ale->port_mask_bits);
> - } else {
> + else
> cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE);
> - }
The code assumed calls cpsw_ale_del_mcast() should have a port mask '0' when
deleting an entry. Do we want to have 'dual' functionality on it?
This will delete mcast entries if port mask is 0 or port mask matches exactly
what's configured right?
> cpsw_ale_write(ale, idx, ale_entry);
> return 0;
> @@ -478,6 +480,10 @@ static void cpsw_ale_del_vlan_modify(struct cpsw_ale *ale, u32 *ale_entry,
> members = cpsw_ale_get_vlan_member_list(ale_entry,
> ale->vlan_field_bits);
> members &= ~port_mask;
> + if (!members) {
> + cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE);
> + return;
> + }
This makes sense the call was missing
>
> untag = cpsw_ale_get_vlan_untag_force(ale_entry,
> ale->vlan_field_bits);
> --
> 2.17.1
>
Thanks
/Ilias
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] net: ethernet: ti: ale: ensure vlan/mdb deleted when no members
2019-11-28 8:21 ` Ilias Apalodimas
@ 2019-11-28 9:38 ` Ilias Apalodimas
2019-11-29 17:58 ` Grygorii Strashko
0 siblings, 1 reply; 4+ messages in thread
From: Ilias Apalodimas @ 2019-11-28 9:38 UTC (permalink / raw)
To: Grygorii Strashko
Cc: netdev, David S . Miller, Andrew Lunn, Sekhar Nori,
Ivan Khoronzhuk, linux-kernel, linux-omap
On Thu, Nov 28, 2019 at 10:21:27AM +0200, Ilias Apalodimas wrote:
> On Wed, Nov 27, 2019 at 05:59:05PM +0200, Grygorii Strashko wrote:
> > The recently updated ALE APIs cpsw_ale_del_mcast() and
> > cpsw_ale_del_vlan_modify() have an issue and will not delete ALE entry even
> > if VLAN/mcast group has no more members. Hence fix it here and delete ALE
> > entry if !port_mask.
> >
> > The issue affected only new cpsw switchdev driver.
> >
> > Fixes: e85c14370783 ("net: ethernet: ti: ale: modify vlan/mdb api for switchdev")
> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > ---
> > drivers/net/ethernet/ti/cpsw_ale.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c
> > index 929f3d3354e3..a5179ecfea05 100644
> > --- a/drivers/net/ethernet/ti/cpsw_ale.c
> > +++ b/drivers/net/ethernet/ti/cpsw_ale.c
> > @@ -396,12 +396,14 @@ int cpsw_ale_del_mcast(struct cpsw_ale *ale, const u8 *addr, int port_mask,
> > if (port_mask) {
> > mcast_members = cpsw_ale_get_port_mask(ale_entry,
> > ale->port_mask_bits);
> > - mcast_members &= ~port_mask;
> > - cpsw_ale_set_port_mask(ale_entry, mcast_members,
> > + port_mask = mcast_members & ~port_mask;
> > + }
> > +
> > + if (port_mask)
> > + cpsw_ale_set_port_mask(ale_entry, port_mask,
> > ale->port_mask_bits);
> > - } else {
> > + else
> > cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE);
> > - }
>
> The code assumed calls cpsw_ale_del_mcast() should have a port mask '0' when
> deleting an entry. Do we want to have 'dual' functionality on it?
> This will delete mcast entries if port mask is 0 or port mask matches exactly
> what's configured right?
>
Deleting the ALE entry if the port_mask matches execlty what's configured makes
sense. Can we change it to something that doesn't change the function argument?
I think something like:
mcast_members = 0;
if (port_mask) {
mcast_members = cpsw_ale_get_port_mask(ale_entry,
ale->port_mask_bits);
mcast_members &= ~port_mask;
}
if (mcast_members)
cpsw_ale_set_port_mask(ale_entry, mcast_members, ....)
else
cpsw_ale_set_entry_type(....)
is more readable?
Thanks
/Ilias
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] net: ethernet: ti: ale: ensure vlan/mdb deleted when no members
2019-11-28 9:38 ` Ilias Apalodimas
@ 2019-11-29 17:58 ` Grygorii Strashko
0 siblings, 0 replies; 4+ messages in thread
From: Grygorii Strashko @ 2019-11-29 17:58 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: netdev, David S . Miller, Andrew Lunn, Sekhar Nori,
Ivan Khoronzhuk, linux-kernel, linux-omap
On 28/11/2019 11:38, Ilias Apalodimas wrote:
> On Thu, Nov 28, 2019 at 10:21:27AM +0200, Ilias Apalodimas wrote:
>> On Wed, Nov 27, 2019 at 05:59:05PM +0200, Grygorii Strashko wrote:
>>> The recently updated ALE APIs cpsw_ale_del_mcast() and
>>> cpsw_ale_del_vlan_modify() have an issue and will not delete ALE entry even
>>> if VLAN/mcast group has no more members. Hence fix it here and delete ALE
>>> entry if !port_mask.
>>>
>>> The issue affected only new cpsw switchdev driver.
>>>
>>> Fixes: e85c14370783 ("net: ethernet: ti: ale: modify vlan/mdb api for switchdev")
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>> drivers/net/ethernet/ti/cpsw_ale.c | 14 ++++++++++----
>>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c
>>> index 929f3d3354e3..a5179ecfea05 100644
>>> --- a/drivers/net/ethernet/ti/cpsw_ale.c
>>> +++ b/drivers/net/ethernet/ti/cpsw_ale.c
>>> @@ -396,12 +396,14 @@ int cpsw_ale_del_mcast(struct cpsw_ale *ale, const u8 *addr, int port_mask,
>>> if (port_mask) {
>>> mcast_members = cpsw_ale_get_port_mask(ale_entry,
>>> ale->port_mask_bits);
>>> - mcast_members &= ~port_mask;
>>> - cpsw_ale_set_port_mask(ale_entry, mcast_members,
>>> + port_mask = mcast_members & ~port_mask;
>>> + }
>>> +
>>> + if (port_mask)
>>> + cpsw_ale_set_port_mask(ale_entry, port_mask,
>>> ale->port_mask_bits);
>>> - } else {
>>> + else
>>> cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE);
>>> - }
>>
>> The code assumed calls cpsw_ale_del_mcast() should have a port mask '0' when
>> deleting an entry. Do we want to have 'dual' functionality on it?
>> This will delete mcast entries if port mask is 0 or port mask matches exactly
>> what's configured right?
>>
>
> Deleting the ALE entry if the port_mask matches execlty what's configured makes
> sense. Can we change it to something that doesn't change the function argument?
>
> I think something like:
> mcast_members = 0;
> if (port_mask) {
> mcast_members = cpsw_ale_get_port_mask(ale_entry,
> ale->port_mask_bits);
> mcast_members &= ~port_mask;
> }
> if (mcast_members)
> cpsw_ale_set_port_mask(ale_entry, mcast_members, ....)
> else
> cpsw_ale_set_entry_type(....)
>
> is more readable?
>
Thank you. I've sent v2.
--
Best regards,
grygorii
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-29 17:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-27 15:59 [PATCH] net: ethernet: ti: ale: ensure vlan/mdb deleted when no members Grygorii Strashko
2019-11-28 8:21 ` Ilias Apalodimas
2019-11-28 9:38 ` Ilias Apalodimas
2019-11-29 17:58 ` Grygorii Strashko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox