* [PATCH net-next,v5] bonding: return -ENOMEM instead of BUG in alb_upper_dev_walk
@ 2023-11-21 12:58 Zhengchao Shao
2023-11-22 2:04 ` shaozhengchao
0 siblings, 1 reply; 4+ messages in thread
From: Zhengchao Shao @ 2023-11-21 12:58 UTC (permalink / raw)
To: netdev, davem, edumazet, kuba, pabeni
Cc: j.vosburgh, andy, weiyongjun1, yuehaibing, shaozhengchao
If failed to allocate "tags" or could not find the final upper device from
start_dev's upper list in bond_verify_device_path(), only the loopback
detection of the current upper device should be affected, and the system is
no need to be panic.
So just return -ENOMEM in alb_upper_dev_walk to stop walking.
I also think that the following function calls
netdev_walk_all_upper_dev_rcu
---->>>alb_upper_dev_walk
---------->>>bond_verify_device_path
From this way, "end device" can eventually be obtained from "start device"
in bond_verify_device_path, IS_ERR(tags) could be instead of
IS_ERR_OR_NULL(tags) in alb_upper_dev_walk.
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
---
v5: drop print information, if the memory allocation fails, the mm layer
will emit a lot warning comprising the backtrace
v4: print information instead of warn
v3: return -ENOMEM instead of zero to stop walk
v2: use WARN_ON_ONCE instead of WARN_ON
---
drivers/net/bonding/bond_alb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index dc2c7b979656..7edf0fd58c34 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -985,7 +985,8 @@ static int alb_upper_dev_walk(struct net_device *upper,
if (netif_is_macvlan(upper) && !strict_match) {
tags = bond_verify_device_path(bond->dev, upper, 0);
if (IS_ERR_OR_NULL(tags))
- BUG();
+ return -ENOMEM;
+
alb_send_lp_vid(slave, upper->dev_addr,
tags[0].vlan_proto, tags[0].vlan_id);
kfree(tags);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next,v5] bonding: return -ENOMEM instead of BUG in alb_upper_dev_walk
2023-11-21 12:58 [PATCH net-next,v5] bonding: return -ENOMEM instead of BUG in alb_upper_dev_walk Zhengchao Shao
@ 2023-11-22 2:04 ` shaozhengchao
2023-11-22 16:16 ` Paolo Abeni
0 siblings, 1 reply; 4+ messages in thread
From: shaozhengchao @ 2023-11-22 2:04 UTC (permalink / raw)
To: netdev, davem, edumazet, kuba, pabeni
Cc: j.vosburgh, andy, weiyongjun1, yuehaibing
On 2023/11/21 20:58, Zhengchao Shao wrote:
> If failed to allocate "tags" or could not find the final upper device from
> start_dev's upper list in bond_verify_device_path(), only the loopback
> detection of the current upper device should be affected, and the system is
> no need to be panic.
> So just return -ENOMEM in alb_upper_dev_walk to stop walking.
>
> I also think that the following function calls
> netdev_walk_all_upper_dev_rcu
> ---->>>alb_upper_dev_walk
> ---------->>>bond_verify_device_path
>>From this way, "end device" can eventually be obtained from "start device"
> in bond_verify_device_path, IS_ERR(tags) could be instead of
> IS_ERR_OR_NULL(tags) in alb_upper_dev_walk.
>
> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
> ---
> v5: drop print information, if the memory allocation fails, the mm layer
> will emit a lot warning comprising the backtrace
> v4: print information instead of warn
> v3: return -ENOMEM instead of zero to stop walk
> v2: use WARN_ON_ONCE instead of WARN_ON
> ---
> drivers/net/bonding/bond_alb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index dc2c7b979656..7edf0fd58c34 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -985,7 +985,8 @@ static int alb_upper_dev_walk(struct net_device *upper,
> if (netif_is_macvlan(upper) && !strict_match) {
> tags = bond_verify_device_path(bond->dev, upper, 0);
> if (IS_ERR_OR_NULL(tags))
> - BUG();
> + return -ENOMEM;
> +
> alb_send_lp_vid(slave, upper->dev_addr,
> tags[0].vlan_proto, tags[0].vlan_id);
> kfree(tags);
Hi Paolo:
I find that v4 has been merged into net-next. So v5 is not
needed. But should I send another patch to drop that print info?
Thanks
Zhengchao Shao
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next,v5] bonding: return -ENOMEM instead of BUG in alb_upper_dev_walk
2023-11-22 2:04 ` shaozhengchao
@ 2023-11-22 16:16 ` Paolo Abeni
2023-11-23 1:38 ` shaozhengchao
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2023-11-22 16:16 UTC (permalink / raw)
To: shaozhengchao, netdev, davem, edumazet, kuba
Cc: j.vosburgh, andy, weiyongjun1, yuehaibing
On Wed, 2023-11-22 at 10:04 +0800, shaozhengchao wrote:
>
> On 2023/11/21 20:58, Zhengchao Shao wrote:
> > If failed to allocate "tags" or could not find the final upper device from
> > start_dev's upper list in bond_verify_device_path(), only the loopback
> > detection of the current upper device should be affected, and the system is
> > no need to be panic.
> > So just return -ENOMEM in alb_upper_dev_walk to stop walking.
> >
> > I also think that the following function calls
> > netdev_walk_all_upper_dev_rcu
> > ---->>>alb_upper_dev_walk
> > ---------->>>bond_verify_device_path
> > > From this way, "end device" can eventually be obtained from "start device"
> > in bond_verify_device_path, IS_ERR(tags) could be instead of
> > IS_ERR_OR_NULL(tags) in alb_upper_dev_walk.
> >
> > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> > Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
> > ---
> > v5: drop print information, if the memory allocation fails, the mm layer
> > will emit a lot warning comprising the backtrace
> > v4: print information instead of warn
> > v3: return -ENOMEM instead of zero to stop walk
> > v2: use WARN_ON_ONCE instead of WARN_ON
> > ---
> > drivers/net/bonding/bond_alb.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> > index dc2c7b979656..7edf0fd58c34 100644
> > --- a/drivers/net/bonding/bond_alb.c
> > +++ b/drivers/net/bonding/bond_alb.c
> > @@ -985,7 +985,8 @@ static int alb_upper_dev_walk(struct net_device *upper,
> > if (netif_is_macvlan(upper) && !strict_match) {
> > tags = bond_verify_device_path(bond->dev, upper, 0);
> > if (IS_ERR_OR_NULL(tags))
> > - BUG();
> > + return -ENOMEM;
> > +
> > alb_send_lp_vid(slave, upper->dev_addr,
> > tags[0].vlan_proto, tags[0].vlan_id);
> > kfree(tags);
> Hi Paolo:
> I find that v4 has been merged into net-next.
I'm sorry, that was due to PEBKAC here.
> So v5 is not
> needed. But should I send another patch to drop that print info?
Yes please, send a follow-up just dropping the print.
Thanks!
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next,v5] bonding: return -ENOMEM instead of BUG in alb_upper_dev_walk
2023-11-22 16:16 ` Paolo Abeni
@ 2023-11-23 1:38 ` shaozhengchao
0 siblings, 0 replies; 4+ messages in thread
From: shaozhengchao @ 2023-11-23 1:38 UTC (permalink / raw)
To: Paolo Abeni, netdev, davem, edumazet, kuba
Cc: j.vosburgh, andy, weiyongjun1, yuehaibing
On 2023/11/23 0:16, Paolo Abeni wrote:
> On Wed, 2023-11-22 at 10:04 +0800, shaozhengchao wrote:
>>
>> On 2023/11/21 20:58, Zhengchao Shao wrote:
>>> If failed to allocate "tags" or could not find the final upper device from
>>> start_dev's upper list in bond_verify_device_path(), only the loopback
>>> detection of the current upper device should be affected, and the system is
>>> no need to be panic.
>>> So just return -ENOMEM in alb_upper_dev_walk to stop walking.
>>>
>>> I also think that the following function calls
>>> netdev_walk_all_upper_dev_rcu
>>> ---->>>alb_upper_dev_walk
>>> ---------->>>bond_verify_device_path
>>>> From this way, "end device" can eventually be obtained from "start device"
>>> in bond_verify_device_path, IS_ERR(tags) could be instead of
>>> IS_ERR_OR_NULL(tags) in alb_upper_dev_walk.
>>>
>>> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
>>> Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
>>> ---
>>> v5: drop print information, if the memory allocation fails, the mm layer
>>> will emit a lot warning comprising the backtrace
>>> v4: print information instead of warn
>>> v3: return -ENOMEM instead of zero to stop walk
>>> v2: use WARN_ON_ONCE instead of WARN_ON
>>> ---
>>> drivers/net/bonding/bond_alb.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>>> index dc2c7b979656..7edf0fd58c34 100644
>>> --- a/drivers/net/bonding/bond_alb.c
>>> +++ b/drivers/net/bonding/bond_alb.c
>>> @@ -985,7 +985,8 @@ static int alb_upper_dev_walk(struct net_device *upper,
>>> if (netif_is_macvlan(upper) && !strict_match) {
>>> tags = bond_verify_device_path(bond->dev, upper, 0);
>>> if (IS_ERR_OR_NULL(tags))
>>> - BUG();
>>> + return -ENOMEM;
>>> +
>>> alb_send_lp_vid(slave, upper->dev_addr,
>>> tags[0].vlan_proto, tags[0].vlan_id);
>>> kfree(tags);
>> Hi Paolo:
>> I find that v4 has been merged into net-next.
>
> I'm sorry, that was due to PEBKAC here.
>
>> So v5 is not
>> needed. But should I send another patch to drop that print info?
>
> Yes please, send a follow-up just dropping the print.
>
OK, I will do it today.
Thanks
Zhengchao Shao
> Thanks!
>
> Paolo
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-11-23 1:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-21 12:58 [PATCH net-next,v5] bonding: return -ENOMEM instead of BUG in alb_upper_dev_walk Zhengchao Shao
2023-11-22 2:04 ` shaozhengchao
2023-11-22 16:16 ` Paolo Abeni
2023-11-23 1:38 ` shaozhengchao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).