Message ID | 20190722140848.8872-1-i.maximets@samsung.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev] netdev-afxdp: Fix use of unconfigured device. | expand |
On Mon, Jul 22, 2019 at 05:08:48PM +0300, Ilya Maximets wrote: > In case of failure of 'xsk_configure_all()', 'n_rxq' and 'xdpmode' > will remain in a new state. This will result in successful > reconfiguration (immediate return, because configuration is already > applied) if 'netdev_reconfigure()' will be called again. > > Same issue was fixed previously for netdev-dpdk using 'dev->started' > flag in commit: > 606f66507250 ("netdev-dpdk: Don't use PMD driver if not configured successfully") > > Let's use similar approach with checking the 'dev->xsks' which only > exists if configuration was successful. > > Additionally implemented 'netdev_afxdp_construct()' function to > explicitly initialize all the specific fields and request the > reconfiguration. > > CC: William Tu <u9012063@gmail.com> > Fixes: 0de1b425962d ("netdev-afxdp: add new netdev type for AF_XDP.") > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> LGTM, thanks for the fix. Acked-by: William Tu <u9012063@gmail.com> <snip>
On 22.07.2019 21:10, William Tu wrote: > On Mon, Jul 22, 2019 at 05:08:48PM +0300, Ilya Maximets wrote: >> In case of failure of 'xsk_configure_all()', 'n_rxq' and 'xdpmode' >> will remain in a new state. This will result in successful >> reconfiguration (immediate return, because configuration is already >> applied) if 'netdev_reconfigure()' will be called again. >> >> Same issue was fixed previously for netdev-dpdk using 'dev->started' >> flag in commit: >> 606f66507250 ("netdev-dpdk: Don't use PMD driver if not configured successfully") >> >> Let's use similar approach with checking the 'dev->xsks' which only >> exists if configuration was successful. >> >> Additionally implemented 'netdev_afxdp_construct()' function to >> explicitly initialize all the specific fields and request the >> reconfiguration. >> >> CC: William Tu <u9012063@gmail.com> >> Fixes: 0de1b425962d ("netdev-afxdp: add new netdev type for AF_XDP.") >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > > LGTM, thanks for the fix. > > Acked-by: William Tu <u9012063@gmail.com> Thanks! Applied to master and branch-2.12. Best regards, Ilya Maximets.
diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c index 80f13fff3..6b0b93e7f 100644 --- a/lib/netdev-afxdp.c +++ b/lib/netdev-afxdp.c @@ -525,7 +525,8 @@ netdev_afxdp_reconfigure(struct netdev *netdev) ovs_mutex_lock(&dev->mutex); if (netdev->n_rxq == dev->requested_n_rxq - && dev->xdpmode == dev->requested_xdpmode) { + && dev->xdpmode == dev->requested_xdpmode + && dev->xsks) { goto out; } @@ -965,6 +966,33 @@ netdev_afxdp_rxq_destruct(struct netdev_rxq *rxq_ OVS_UNUSED) /* Nothing. */ } +int +netdev_afxdp_construct(struct netdev *netdev) +{ + struct netdev_linux *dev = netdev_linux_cast(netdev); + int ret; + + /* Configure common netdev-linux first. */ + ret = netdev_linux_construct(netdev); + if (ret) { + return ret; + } + + /* Queues should not be used before the first reconfiguration. Clearing. */ + netdev->n_rxq = 0; + netdev->n_txq = 0; + dev->xdpmode = 0; + + dev->requested_n_rxq = NR_QUEUE; + dev->requested_xdpmode = XDP_COPY; + + dev->xsks = NULL; + dev->tx_locks = NULL; + + netdev_request_reconfigure(netdev); + return 0; +} + void netdev_afxdp_destruct(struct netdev *netdev) { diff --git a/lib/netdev-afxdp.h b/lib/netdev-afxdp.h index ea63d76cf..6a72bd9a8 100644 --- a/lib/netdev-afxdp.h +++ b/lib/netdev-afxdp.h @@ -36,6 +36,7 @@ struct netdev_stats; int netdev_afxdp_rxq_construct(struct netdev_rxq *rxq_); void netdev_afxdp_rxq_destruct(struct netdev_rxq *rxq_); +int netdev_afxdp_construct(struct netdev *netdev_); void netdev_afxdp_destruct(struct netdev *netdev_); int netdev_afxdp_rxq_recv(struct netdev_rxq *rxq_, diff --git a/lib/netdev-linux-private.h b/lib/netdev-linux-private.h index ba87f9718..a350be151 100644 --- a/lib/netdev-linux-private.h +++ b/lib/netdev-linux-private.h @@ -43,6 +43,7 @@ struct netdev_rxq_linux { int fd; }; +int netdev_linux_construct(struct netdev *); void netdev_linux_run(const struct netdev_class *); int get_stats_via_netlink(const struct netdev *netdev_, diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 877049508..2432cd176 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -906,7 +906,7 @@ netdev_linux_common_construct(struct netdev *netdev_) } /* Creates system and internal devices. */ -static int +int netdev_linux_construct(struct netdev *netdev_) { struct netdev_linux *netdev = netdev_linux_cast(netdev_); @@ -3290,7 +3290,7 @@ const struct netdev_class netdev_afxdp_class = { NETDEV_LINUX_CLASS_COMMON, .type = "afxdp", .is_pmd = true, - .construct = netdev_linux_construct, + .construct = netdev_afxdp_construct, .destruct = netdev_afxdp_destruct, .get_stats = netdev_afxdp_get_stats, .get_status = netdev_linux_get_status,
In case of failure of 'xsk_configure_all()', 'n_rxq' and 'xdpmode' will remain in a new state. This will result in successful reconfiguration (immediate return, because configuration is already applied) if 'netdev_reconfigure()' will be called again. Same issue was fixed previously for netdev-dpdk using 'dev->started' flag in commit: 606f66507250 ("netdev-dpdk: Don't use PMD driver if not configured successfully") Let's use similar approach with checking the 'dev->xsks' which only exists if configuration was successful. Additionally implemented 'netdev_afxdp_construct()' function to explicitly initialize all the specific fields and request the reconfiguration. CC: William Tu <u9012063@gmail.com> Fixes: 0de1b425962d ("netdev-afxdp: add new netdev type for AF_XDP.") Signed-off-by: Ilya Maximets <i.maximets@samsung.com> --- lib/netdev-afxdp.c | 30 +++++++++++++++++++++++++++++- lib/netdev-afxdp.h | 1 + lib/netdev-linux-private.h | 1 + lib/netdev-linux.c | 4 ++-- 4 files changed, 33 insertions(+), 3 deletions(-)