[ovs-dev] netdev-afxdp: Fix use of unconfigured device.
diff mbox series

Message ID 20190722140848.8872-1-i.maximets@samsung.com
State Accepted
Headers show
Series
  • [ovs-dev] netdev-afxdp: Fix use of unconfigured device.
Related show

Commit Message

Ilya Maximets July 22, 2019, 2:08 p.m. UTC
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(-)

Comments

William Tu July 22, 2019, 6:10 p.m. UTC | #1
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>
Ilya Maximets July 23, 2019, 7:56 a.m. UTC | #2
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.

Patch
diff mbox series

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,