* [PATCH net-next,v2] bonding: use WARN_ON_ONCE instead of BUG in alb_upper_dev_walk
@ 2023-11-14 9:18 Zhengchao Shao
2023-11-14 10:16 ` Hangbin Liu
2023-11-14 19:40 ` Jay Vosburgh
0 siblings, 2 replies; 5+ messages in thread
From: Zhengchao Shao @ 2023-11-14 9:18 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.
Using WARN_ON_ONCE here is to avoid spamming the log if there's a lot of
macvlans above the bond.
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
v2: use WARN_ON_ONCE instead of WARN_ON
---
drivers/net/bonding/bond_alb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index dc2c7b979656..a7bad0fff8cb 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -984,8 +984,10 @@ 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();
+ if (IS_ERR_OR_NULL(tags)) {
+ WARN_ON_ONCE(1);
+ return 0;
+ }
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] 5+ messages in thread* Re: [PATCH net-next,v2] bonding: use WARN_ON_ONCE instead of BUG in alb_upper_dev_walk
2023-11-14 9:18 [PATCH net-next,v2] bonding: use WARN_ON_ONCE instead of BUG in alb_upper_dev_walk Zhengchao Shao
@ 2023-11-14 10:16 ` Hangbin Liu
2023-11-14 12:38 ` shaozhengchao
2023-11-14 19:40 ` Jay Vosburgh
1 sibling, 1 reply; 5+ messages in thread
From: Hangbin Liu @ 2023-11-14 10:16 UTC (permalink / raw)
To: Zhengchao Shao
Cc: netdev, davem, edumazet, kuba, pabeni, j.vosburgh, andy,
weiyongjun1, yuehaibing
On Tue, Nov 14, 2023 at 05:18:29PM +0800, 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.
> Using WARN_ON_ONCE here is to avoid spamming the log if there's a lot of
> macvlans above the bond.
>
> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> ---
> v2: use WARN_ON_ONCE instead of WARN_ON
> ---
> drivers/net/bonding/bond_alb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index dc2c7b979656..a7bad0fff8cb 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -984,8 +984,10 @@ 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();
> + if (IS_ERR_OR_NULL(tags)) {
> + WARN_ON_ONCE(1);
> + return 0;
Return 0 for an error looks weird. Should we keep walking the list if
allocate "tags" failed?
Thanks
Hangbin
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net-next,v2] bonding: use WARN_ON_ONCE instead of BUG in alb_upper_dev_walk
2023-11-14 10:16 ` Hangbin Liu
@ 2023-11-14 12:38 ` shaozhengchao
0 siblings, 0 replies; 5+ messages in thread
From: shaozhengchao @ 2023-11-14 12:38 UTC (permalink / raw)
To: Hangbin Liu
Cc: netdev, davem, edumazet, kuba, pabeni, j.vosburgh, andy,
weiyongjun1, yuehaibing
On 2023/11/14 18:16, Hangbin Liu wrote:
> On Tue, Nov 14, 2023 at 05:18:29PM +0800, 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.
>> Using WARN_ON_ONCE here is to avoid spamming the log if there's a lot of
>> macvlans above the bond.
>>
>> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
>> ---
>> v2: use WARN_ON_ONCE instead of WARN_ON
>> ---
>> drivers/net/bonding/bond_alb.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index dc2c7b979656..a7bad0fff8cb 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -984,8 +984,10 @@ 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();
>> + if (IS_ERR_OR_NULL(tags)) {
>> + WARN_ON_ONCE(1);
>> + return 0;
>
> Return 0 for an error looks weird. Should we keep walking the list if
> allocate "tags" failed?
>
> Thanks
> Hangbin
>
Hi Hangbin:
I think minimizing the impact of a single allocation failure
is OK.
Zhengchao Shao
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next,v2] bonding: use WARN_ON_ONCE instead of BUG in alb_upper_dev_walk
2023-11-14 9:18 [PATCH net-next,v2] bonding: use WARN_ON_ONCE instead of BUG in alb_upper_dev_walk Zhengchao Shao
2023-11-14 10:16 ` Hangbin Liu
@ 2023-11-14 19:40 ` Jay Vosburgh
2023-11-15 11:24 ` shaozhengchao
1 sibling, 1 reply; 5+ messages in thread
From: Jay Vosburgh @ 2023-11-14 19:40 UTC (permalink / raw)
To: Zhengchao Shao
Cc: netdev, davem, edumazet, kuba, pabeni, andy, weiyongjun1,
yuehaibing
Zhengchao Shao <shaozhengchao@huawei.com> 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.
>Using WARN_ON_ONCE here is to avoid spamming the log if there's a lot of
>macvlans above the bond.
>
>Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
>---
>v2: use WARN_ON_ONCE instead of WARN_ON
>---
> drivers/net/bonding/bond_alb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index dc2c7b979656..a7bad0fff8cb 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -984,8 +984,10 @@ 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();
>+ if (IS_ERR_OR_NULL(tags)) {
>+ WARN_ON_ONCE(1);
>+ return 0;
Ok, I know this is what I said, but on reflection, I think this
should really return non-zero to terminate the device walk.
-J
>+ }
> alb_send_lp_vid(slave, upper->dev_addr,
> tags[0].vlan_proto, tags[0].vlan_id);
> kfree(tags);
>--
>2.34.1
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net-next,v2] bonding: use WARN_ON_ONCE instead of BUG in alb_upper_dev_walk
2023-11-14 19:40 ` Jay Vosburgh
@ 2023-11-15 11:24 ` shaozhengchao
0 siblings, 0 replies; 5+ messages in thread
From: shaozhengchao @ 2023-11-15 11:24 UTC (permalink / raw)
To: Jay Vosburgh
Cc: netdev, davem, edumazet, kuba, pabeni, andy, weiyongjun1,
yuehaibing
On 2023/11/15 3:40, Jay Vosburgh wrote:
> Zhengchao Shao <shaozhengchao@huawei.com> 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.
>> Using WARN_ON_ONCE here is to avoid spamming the log if there's a lot of
>> macvlans above the bond.
>>
>> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
>> ---
>> v2: use WARN_ON_ONCE instead of WARN_ON
>> ---
>> drivers/net/bonding/bond_alb.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index dc2c7b979656..a7bad0fff8cb 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -984,8 +984,10 @@ 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();
>> + if (IS_ERR_OR_NULL(tags)) {
>> + WARN_ON_ONCE(1);
>> + return 0;
>
> Ok, I know this is what I said, but on reflection, I think this
> should really return non-zero to terminate the device walk.
>
> -J
>
After this failure, there is a high probability that walk
process will still fail. Therefore, it is OK to exit directly. Thanks.
I will send V3.
Zhengchao Shao
>
>> + }
>> alb_send_lp_vid(slave, upper->dev_addr,
>> tags[0].vlan_proto, tags[0].vlan_id);
>> kfree(tags);
>> --
>> 2.34.1
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-15 11:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-14 9:18 [PATCH net-next,v2] bonding: use WARN_ON_ONCE instead of BUG in alb_upper_dev_walk Zhengchao Shao
2023-11-14 10:16 ` Hangbin Liu
2023-11-14 12:38 ` shaozhengchao
2023-11-14 19:40 ` Jay Vosburgh
2023-11-15 11:24 ` 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).