| Message ID | 20220923154556.721511-8-ioana.ciornei@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dpaa2-eth: AF_XDP zero-copy support | 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 | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 7 of 7 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/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 24 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Fri, 2022-09-23 at 18:45 +0300, Ioana Ciornei wrote: > Instead of calling the internal functions which implement .ndo_stop and > .ndo_open, we can simply call dev_close and dev_open, so that we keep > the code cleaner. > > Also, in the next patches we'll use the same APIs from other files > without needing to export the internal functions. > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> This has the not so nice side effect that in case of dev_open() error, the device will flip status after dpaa2_eth_setup_xdp(). We should try to avoid that. I think it's better if you export the helper instead (or even better, do something more low level-cant-fail like stop the relevant h/w queue, reconfigure, restart the h/w queue). Cheers, Paolo
On Tue, Sep 27, 2022 at 02:54:58PM +0200, Paolo Abeni wrote: > On Fri, 2022-09-23 at 18:45 +0300, Ioana Ciornei wrote: > > Instead of calling the internal functions which implement .ndo_stop and > > .ndo_open, we can simply call dev_close and dev_open, so that we keep > > the code cleaner. > > > > Also, in the next patches we'll use the same APIs from other files > > without needing to export the internal functions. > > > > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> > > This has the not so nice side effect that in case of dev_open() error, > the device will flip status after dpaa2_eth_setup_xdp(). We should try > to avoid that. > > I think it's better if you export the helper instead (or even better, > do something more low level-cant-fail like stop the relevant h/w queue, > reconfigure, restart the h/w queue). > I would also have wanted to not have to stop the entire interface but unfortunately this is not possible. In order to change which buffer pool is used on which queue I have to stop the interface (dpni_disable() ) which would flip the link. I can export the helpers but this would not change the behavior since the dpaa2_eth_stop() and dpaa2_eth_open() are the exact callbacks setup for .ndo_stop() and .ndo_open(). Ioana
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c index aa93ed339b63..d40d25e62b6a 100644 --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c @@ -2626,7 +2626,7 @@ static int dpaa2_eth_setup_xdp(struct net_device *dev, struct bpf_prog *prog) need_update = (!!priv->xdp_prog != !!prog); if (up) - dpaa2_eth_stop(dev); + dev_close(dev); /* While in xdp mode, enforce a maximum Rx frame size based on MTU. * Also, when switching between xdp/non-xdp modes we need to reconfigure @@ -2654,7 +2654,7 @@ static int dpaa2_eth_setup_xdp(struct net_device *dev, struct bpf_prog *prog) } if (up) { - err = dpaa2_eth_open(dev); + err = dev_open(dev, NULL); if (err) return err; } @@ -2665,7 +2665,7 @@ static int dpaa2_eth_setup_xdp(struct net_device *dev, struct bpf_prog *prog) if (prog) bpf_prog_sub(prog, priv->num_channels); if (up) - dpaa2_eth_open(dev); + dev_open(dev, NULL); return err; }
Instead of calling the internal functions which implement .ndo_stop and .ndo_open, we can simply call dev_close and dev_open, so that we keep the code cleaner. Also, in the next patches we'll use the same APIs from other files without needing to export the internal functions. Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com> --- Changes in v2: - none drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)