| Message ID | 20220307125735.GC16710@kili (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8daf4e75fc09d6b0ca8fea0988959c99643aa8a8 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] vxlan_core: delete unnecessary condition | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/checkpatch | warning | WARNING: line length of 85 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On 07/03/2022 14:57, Dan Carpenter wrote: > The previous check handled the "if (!nh)" condition so we know "nh" > is non-NULL here. Delete the check and pull the code in one tab. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > This not a bug so a Fixes tag is innappropriate, however for reviewers > this was introduced in commit 4095e0e1328a ("drivers: vxlan: vnifilter: > per vni stats") No, it was not introduced by that commit. It was introduced by commit: 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries") > --- > drivers/net/vxlan/vxlan_core.c | 54 ++++++++++++++++------------------ > 1 file changed, 26 insertions(+), 28 deletions(-) > > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c > index 4ab09dd5a32a..795f438940ee 100644 > --- a/drivers/net/vxlan/vxlan_core.c > +++ b/drivers/net/vxlan/vxlan_core.c > @@ -811,37 +811,35 @@ static int vxlan_fdb_nh_update(struct vxlan_dev *vxlan, struct vxlan_fdb *fdb, > goto err_inval; > } > > - if (nh) { > - if (!nexthop_get(nh)) { > - NL_SET_ERR_MSG(extack, "Nexthop has been deleted"); > - nh = NULL; > - goto err_inval; > - } > - if (!nexthop_is_fdb(nh)) { > - NL_SET_ERR_MSG(extack, "Nexthop is not a fdb nexthop"); > - goto err_inval; > - } > + if (!nexthop_get(nh)) { > + NL_SET_ERR_MSG(extack, "Nexthop has been deleted"); > + nh = NULL; > + goto err_inval; > + } > + if (!nexthop_is_fdb(nh)) { > + NL_SET_ERR_MSG(extack, "Nexthop is not a fdb nexthop"); > + goto err_inval; > + } > > - if (!nexthop_is_multipath(nh)) { > - NL_SET_ERR_MSG(extack, "Nexthop is not a multipath group"); > + if (!nexthop_is_multipath(nh)) { > + NL_SET_ERR_MSG(extack, "Nexthop is not a multipath group"); > + goto err_inval; > + } > + > + /* check nexthop group family */ > + switch (vxlan->default_dst.remote_ip.sa.sa_family) { > + case AF_INET: > + if (!nexthop_has_v4(nh)) { > + err = -EAFNOSUPPORT; > + NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); > goto err_inval; > } > - > - /* check nexthop group family */ > - switch (vxlan->default_dst.remote_ip.sa.sa_family) { > - case AF_INET: > - if (!nexthop_has_v4(nh)) { > - err = -EAFNOSUPPORT; > - NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); > - goto err_inval; > - } > - break; > - case AF_INET6: > - if (nexthop_has_v4(nh)) { > - err = -EAFNOSUPPORT; > - NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); > - goto err_inval; > - } > + break; > + case AF_INET6: > + if (nexthop_has_v4(nh)) { > + err = -EAFNOSUPPORT; > + NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); > + goto err_inval; > } > } >
On 07/03/2022 15:05, Nikolay Aleksandrov wrote: > On 07/03/2022 14:57, Dan Carpenter wrote: >> The previous check handled the "if (!nh)" condition so we know "nh" >> is non-NULL here. Delete the check and pull the code in one tab. >> >> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >> --- >> This not a bug so a Fixes tag is innappropriate, however for reviewers >> this was introduced in commit 4095e0e1328a ("drivers: vxlan: vnifilter: >> per vni stats") > > No, it was not introduced by that commit. > It was introduced by commit: > 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries") > Forgot to add: patch looks good to me. :) Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org> Thanks, Nik >> --- >> drivers/net/vxlan/vxlan_core.c | 54 ++++++++++++++++------------------ >> 1 file changed, 26 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c >> index 4ab09dd5a32a..795f438940ee 100644 >> --- a/drivers/net/vxlan/vxlan_core.c >> +++ b/drivers/net/vxlan/vxlan_core.c >> @@ -811,37 +811,35 @@ static int vxlan_fdb_nh_update(struct vxlan_dev *vxlan, struct vxlan_fdb *fdb, >> goto err_inval; >> } >> >> - if (nh) { >> - if (!nexthop_get(nh)) { >> - NL_SET_ERR_MSG(extack, "Nexthop has been deleted"); >> - nh = NULL; >> - goto err_inval; >> - } >> - if (!nexthop_is_fdb(nh)) { >> - NL_SET_ERR_MSG(extack, "Nexthop is not a fdb nexthop"); >> - goto err_inval; >> - } >> + if (!nexthop_get(nh)) { >> + NL_SET_ERR_MSG(extack, "Nexthop has been deleted"); >> + nh = NULL; >> + goto err_inval; >> + } >> + if (!nexthop_is_fdb(nh)) { >> + NL_SET_ERR_MSG(extack, "Nexthop is not a fdb nexthop"); >> + goto err_inval; >> + } >> >> - if (!nexthop_is_multipath(nh)) { >> - NL_SET_ERR_MSG(extack, "Nexthop is not a multipath group"); >> + if (!nexthop_is_multipath(nh)) { >> + NL_SET_ERR_MSG(extack, "Nexthop is not a multipath group"); >> + goto err_inval; >> + } >> + >> + /* check nexthop group family */ >> + switch (vxlan->default_dst.remote_ip.sa.sa_family) { >> + case AF_INET: >> + if (!nexthop_has_v4(nh)) { >> + err = -EAFNOSUPPORT; >> + NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); >> goto err_inval; >> } >> - >> - /* check nexthop group family */ >> - switch (vxlan->default_dst.remote_ip.sa.sa_family) { >> - case AF_INET: >> - if (!nexthop_has_v4(nh)) { >> - err = -EAFNOSUPPORT; >> - NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); >> - goto err_inval; >> - } >> - break; >> - case AF_INET6: >> - if (nexthop_has_v4(nh)) { >> - err = -EAFNOSUPPORT; >> - NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); >> - goto err_inval; >> - } >> + break; >> + case AF_INET6: >> + if (nexthop_has_v4(nh)) { >> + err = -EAFNOSUPPORT; >> + NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); >> + goto err_inval; >> } >> } >> >
On Mon, Mar 07, 2022 at 03:05:49PM +0200, Nikolay Aleksandrov wrote: > On 07/03/2022 14:57, Dan Carpenter wrote: > > The previous check handled the "if (!nh)" condition so we know "nh" > > is non-NULL here. Delete the check and pull the code in one tab. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > This not a bug so a Fixes tag is innappropriate, however for reviewers > > this was introduced in commit 4095e0e1328a ("drivers: vxlan: vnifilter: > > per vni stats") > > No, it was not introduced by that commit. > It was introduced by commit: > 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries") Yeah. Sorry. No idea how I got that wrong. regards, dan carpenter
On 3/7/22 05:07, Nikolay Aleksandrov wrote: > On 07/03/2022 15:05, Nikolay Aleksandrov wrote: >> On 07/03/2022 14:57, Dan Carpenter wrote: >>> The previous check handled the "if (!nh)" condition so we know "nh" >>> is non-NULL here. Delete the check and pull the code in one tab. >>> >>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>> --- >>> This not a bug so a Fixes tag is innappropriate, however for reviewers >>> this was introduced in commit 4095e0e1328a ("drivers: vxlan: vnifilter: >>> per vni stats") >> No, it was not introduced by that commit. >> It was introduced by commit: >> 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries") >> > Forgot to add: patch looks good to me. :) > Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org> +1 Reviewed-by: Roopa Prabhu <roopa@nvidia.com> LGTM too, thanks Dan
Hello: This patch was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Mon, 7 Mar 2022 15:57:36 +0300 you wrote: > The previous check handled the "if (!nh)" condition so we know "nh" > is non-NULL here. Delete the check and pull the code in one tab. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > This not a bug so a Fixes tag is innappropriate, however for reviewers > this was introduced in commit 4095e0e1328a ("drivers: vxlan: vnifilter: > per vni stats") > > [...] Here is the summary with links: - [net-next] vxlan_core: delete unnecessary condition https://git.kernel.org/netdev/net-next/c/8daf4e75fc09 You are awesome, thank you!
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c index 4ab09dd5a32a..795f438940ee 100644 --- a/drivers/net/vxlan/vxlan_core.c +++ b/drivers/net/vxlan/vxlan_core.c @@ -811,37 +811,35 @@ static int vxlan_fdb_nh_update(struct vxlan_dev *vxlan, struct vxlan_fdb *fdb, goto err_inval; } - if (nh) { - if (!nexthop_get(nh)) { - NL_SET_ERR_MSG(extack, "Nexthop has been deleted"); - nh = NULL; - goto err_inval; - } - if (!nexthop_is_fdb(nh)) { - NL_SET_ERR_MSG(extack, "Nexthop is not a fdb nexthop"); - goto err_inval; - } + if (!nexthop_get(nh)) { + NL_SET_ERR_MSG(extack, "Nexthop has been deleted"); + nh = NULL; + goto err_inval; + } + if (!nexthop_is_fdb(nh)) { + NL_SET_ERR_MSG(extack, "Nexthop is not a fdb nexthop"); + goto err_inval; + } - if (!nexthop_is_multipath(nh)) { - NL_SET_ERR_MSG(extack, "Nexthop is not a multipath group"); + if (!nexthop_is_multipath(nh)) { + NL_SET_ERR_MSG(extack, "Nexthop is not a multipath group"); + goto err_inval; + } + + /* check nexthop group family */ + switch (vxlan->default_dst.remote_ip.sa.sa_family) { + case AF_INET: + if (!nexthop_has_v4(nh)) { + err = -EAFNOSUPPORT; + NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); goto err_inval; } - - /* check nexthop group family */ - switch (vxlan->default_dst.remote_ip.sa.sa_family) { - case AF_INET: - if (!nexthop_has_v4(nh)) { - err = -EAFNOSUPPORT; - NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); - goto err_inval; - } - break; - case AF_INET6: - if (nexthop_has_v4(nh)) { - err = -EAFNOSUPPORT; - NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); - goto err_inval; - } + break; + case AF_INET6: + if (nexthop_has_v4(nh)) { + err = -EAFNOSUPPORT; + NL_SET_ERR_MSG(extack, "Nexthop group family not supported"); + goto err_inval; } }
The previous check handled the "if (!nh)" condition so we know "nh" is non-NULL here. Delete the check and pull the code in one tab. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- This not a bug so a Fixes tag is innappropriate, however for reviewers this was introduced in commit 4095e0e1328a ("drivers: vxlan: vnifilter: per vni stats") --- drivers/net/vxlan/vxlan_core.c | 54 ++++++++++++++++------------------ 1 file changed, 26 insertions(+), 28 deletions(-)