| Message ID | 20240918083545.9591-1-jiwonaid0@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] bonding: Add net_ratelimit for bond_xdp_get_xmit_slave in bond_main.c | expand |
On 18/09/2024 11:35, Jiwon Kim wrote: > Add net_ratelimit to reduce warnings and logs. > This addresses the WARNING in bond_xdp_get_xmit_slave reported by syzbot. > This commit message is severely lacking. I did the heavy lifting and gave you detailed analysis of the problem, please describe the actual issue and why this is ok to do. Also the subject is confusing, it should give a concise summary of what the patch is trying to do and please don't include filenames in it. You can take a look at other commits for examples. > Setup: > # Need xdp_tx_prog with return XDP_TX; > ip l add veth0 type veth peer veth1 > ip l add veth3 type veth peer veth4 > ip l add bond0 type bond mode 6 # <- BOND_MODE_ALB, unsupported by xdp > ip l add bond1 type bond # <- BOND_MODE_ROUNDROBIN by default > ip l set veth0 master bond1 > ip l set bond1 up > ip l set dev bond1 xdpdrv object tx_xdp.o section xdp_tx > ip l set veth3 master bond0 > ip l set bond0 up > ip l set veth4 up > ip l set veth3 xdpgeneric object tx_xdp.o section xdp_tx Care to explain why this setup would trigger anything? > > Reported-by: syzbot+c187823a52ed505b2257@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=c187823a52ed505b2257 > Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver") > Signed-off-by: Jiwon Kim <jiwonaid0@gmail.com> > --- > v2: Change the patch to fix bond_xdp_get_xmit_slave > --- > drivers/net/bonding/bond_main.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > index b560644ee1b1..91b9cbdcf274 100644 > --- a/drivers/net/bonding/bond_main.c > +++ b/drivers/net/bonding/bond_main.c > @@ -5610,9 +5610,12 @@ bond_xdp_get_xmit_slave(struct net_device *bond_dev, struct xdp_buff *xdp) > break; > > default: > - /* Should never happen. Mode guarded by bond_xdp_check() */ > - netdev_err(bond_dev, "Unknown bonding mode %d for xdp xmit\n", BOND_MODE(bond)); > - WARN_ON_ONCE(1); > + /* This might occur when a bond device increases bpf_master_redirect_enabled_key, > + * and another bond device with XDP_TX and bond slave. > + */ The comment is confusing and needs to be reworded or dropped altogether. > + if (net_ratelimit()) > + netdev_err(bond_dev, "Unknown bonding mode %d for xdp xmit\n", > + BOND_MODE(bond)); > return NULL; > } >
On Wed, Sep 18, 2024 at 6:51 PM Nikolay Aleksandrov <razor@blackwall.org> wrote: > > On 18/09/2024 11:35, Jiwon Kim wrote: > > Add net_ratelimit to reduce warnings and logs. > > This addresses the WARNING in bond_xdp_get_xmit_slave reported by syzbot. > > > > This commit message is severely lacking. I did the heavy lifting and gave you > detailed analysis of the problem, please describe the actual issue and why > this is ok to do. Also the subject is confusing, it should give a concise > summary of what the patch is trying to do and please don't include filenames in it. > You can take a look at other commits for examples. > > > Setup: > > # Need xdp_tx_prog with return XDP_TX; > > ip l add veth0 type veth peer veth1 > > ip l add veth3 type veth peer veth4 > > ip l add bond0 type bond mode 6 # <- BOND_MODE_ALB, unsupported by xdp > > ip l add bond1 type bond # <- BOND_MODE_ROUNDROBIN by default > > ip l set veth0 master bond1 > > ip l set bond1 up > > ip l set dev bond1 xdpdrv object tx_xdp.o section xdp_tx > > ip l set veth3 master bond0 > > ip l set bond0 up > > ip l set veth4 up > > ip l set veth3 xdpgeneric object tx_xdp.o section xdp_tx > > Care to explain why this setup would trigger anything? > > > > > Reported-by: syzbot+c187823a52ed505b2257@syzkaller.appspotmail.com > > Closes: https://syzkaller.appspot.com/bug?extid=c187823a52ed505b2257 > > Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver") > > Signed-off-by: Jiwon Kim <jiwonaid0@gmail.com> > > --- > > v2: Change the patch to fix bond_xdp_get_xmit_slave > > --- > > drivers/net/bonding/bond_main.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > > index b560644ee1b1..91b9cbdcf274 100644 > > --- a/drivers/net/bonding/bond_main.c > > +++ b/drivers/net/bonding/bond_main.c > > @@ -5610,9 +5610,12 @@ bond_xdp_get_xmit_slave(struct net_device *bond_dev, struct xdp_buff *xdp) > > break; > > > > default: > > - /* Should never happen. Mode guarded by bond_xdp_check() */ > > - netdev_err(bond_dev, "Unknown bonding mode %d for xdp xmit\n", BOND_MODE(bond)); > > - WARN_ON_ONCE(1); > > + /* This might occur when a bond device increases bpf_master_redirect_enabled_key, > > + * and another bond device with XDP_TX and bond slave. > > + */ > > The comment is confusing and needs to be reworded or dropped altogether. > > > + if (net_ratelimit()) > > + netdev_err(bond_dev, "Unknown bonding mode %d for xdp xmit\n", > > + BOND_MODE(bond)); > > return NULL; > > } > > > Hi Nikolay, I have taken the time to review your feedback and have sent [PATCH net v3] for your consideration. Please take a look when you have a moment. Thank you so much! Sincerely, Jiwon Kim
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index b560644ee1b1..91b9cbdcf274 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -5610,9 +5610,12 @@ bond_xdp_get_xmit_slave(struct net_device *bond_dev, struct xdp_buff *xdp) break; default: - /* Should never happen. Mode guarded by bond_xdp_check() */ - netdev_err(bond_dev, "Unknown bonding mode %d for xdp xmit\n", BOND_MODE(bond)); - WARN_ON_ONCE(1); + /* This might occur when a bond device increases bpf_master_redirect_enabled_key, + * and another bond device with XDP_TX and bond slave. + */ + if (net_ratelimit()) + netdev_err(bond_dev, "Unknown bonding mode %d for xdp xmit\n", + BOND_MODE(bond)); return NULL; }
Add net_ratelimit to reduce warnings and logs. This addresses the WARNING in bond_xdp_get_xmit_slave reported by syzbot. Setup: # Need xdp_tx_prog with return XDP_TX; ip l add veth0 type veth peer veth1 ip l add veth3 type veth peer veth4 ip l add bond0 type bond mode 6 # <- BOND_MODE_ALB, unsupported by xdp ip l add bond1 type bond # <- BOND_MODE_ROUNDROBIN by default ip l set veth0 master bond1 ip l set bond1 up ip l set dev bond1 xdpdrv object tx_xdp.o section xdp_tx ip l set veth3 master bond0 ip l set bond0 up ip l set veth4 up ip l set veth3 xdpgeneric object tx_xdp.o section xdp_tx Reported-by: syzbot+c187823a52ed505b2257@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=c187823a52ed505b2257 Fixes: 9e2ee5c7e7c3 ("net, bonding: Add XDP support to the bonding driver") Signed-off-by: Jiwon Kim <jiwonaid0@gmail.com> --- v2: Change the patch to fix bond_xdp_get_xmit_slave --- drivers/net/bonding/bond_main.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)