| Message ID | d563de401d6fdc1c52959300eebb2bbb27c6c181.1674481435.git.leon@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Convert drivers to return XFRM configuration errors through extack | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next, async |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | fail | Errors and warnings before: 12 this patch: 12 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | fail | Errors and warnings before: 10 this patch: 10 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 12 this patch: 12 |
| netdev/checkpatch | warning | WARNING: line length of 83 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Leon Romanovsky <leon@kernel.org> wrote: >From: Leon Romanovsky <leonro@nvidia.com> > >Rely on extack to return failure reason. > >Signed-off-by: Leon Romanovsky <leonro@nvidia.com> >Signed-off-by: Leon Romanovsky <leon@kernel.org> >--- > drivers/net/bonding/bond_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index 686b2a6fd674..00646aa315c3 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -444,7 +444,7 @@ static int bond_ipsec_add_sa(struct xfrm_state *xs, > if (!slave->dev->xfrmdev_ops || > !slave->dev->xfrmdev_ops->xdo_dev_state_add || > netif_is_bond_master(slave->dev)) { >- slave_warn(bond_dev, slave->dev, "Slave does not support ipsec offload\n"); >+ NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload"); > rcu_read_unlock(); > return -EINVAL; > } Why only this one, and not include the other similar slave_warn() calls in the bond_ipsec_* functions? That would seem to make some failures show up in dmesg, and others returned to the caller via extack. -J --- -Jay Vosburgh, jay.vosburgh@canonical.com
On Mon, Jan 23, 2023 at 03:01:32PM -0800, Jay Vosburgh wrote: > Leon Romanovsky <leon@kernel.org> wrote: > > >From: Leon Romanovsky <leonro@nvidia.com> > > > >Rely on extack to return failure reason. > > > >Signed-off-by: Leon Romanovsky <leonro@nvidia.com> > >Signed-off-by: Leon Romanovsky <leon@kernel.org> > >--- > > drivers/net/bonding/bond_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c > >index 686b2a6fd674..00646aa315c3 100644 > >--- a/drivers/net/bonding/bond_main.c > >+++ b/drivers/net/bonding/bond_main.c > >@@ -444,7 +444,7 @@ static int bond_ipsec_add_sa(struct xfrm_state *xs, > > if (!slave->dev->xfrmdev_ops || > > !slave->dev->xfrmdev_ops->xdo_dev_state_add || > > netif_is_bond_master(slave->dev)) { > >- slave_warn(bond_dev, slave->dev, "Slave does not support ipsec offload\n"); > >+ NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload"); > > rcu_read_unlock(); > > return -EINVAL; > > } > > Why only this one, and not include the other similar > slave_warn() calls in the bond_ipsec_* functions? Which functions did you have in mind? The extack was added to XFRM .xdo_dev_state_add() call, which is translated to bond_ipsec_add_sa() with only one slave_warn() print. If you are talking about bond_ipsec_add_sa_all(), that function isn't directly connected to netlink and doesn't have extack pointer to fill. If you are talking about bond_ipsec_del_sai*() and slave_warn() there, it will be better to be deleted/changed to make sure what ipsec_list have only valid devices. Thanks > That would seem to make some failures show up in dmesg, > and others returned to the caller via extack. > > -J > > --- > -Jay Vosburgh, jay.vosburgh@canonical.com
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 686b2a6fd674..00646aa315c3 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -444,7 +444,7 @@ static int bond_ipsec_add_sa(struct xfrm_state *xs, if (!slave->dev->xfrmdev_ops || !slave->dev->xfrmdev_ops->xdo_dev_state_add || netif_is_bond_master(slave->dev)) { - slave_warn(bond_dev, slave->dev, "Slave does not support ipsec offload\n"); + NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload"); rcu_read_unlock(); return -EINVAL; }