diff mbox series

[ovs-dev] netdev-dpdk: Clean up all marker flags if no offloads requested.

Message ID 20240311183231.37253-1-i.maximets@ovn.org
State Accepted
Commit 7df30c86ce12833cf7f9bfc71c166c34692ceff4
Headers show
Series [ovs-dev] netdev-dpdk: Clean up all marker flags if no offloads requested. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Ilya Maximets March 11, 2024, 6:32 p.m. UTC
Some drivers (primarily, Intel ones) do not expect any marking flags
being set if no offloads are requested.  If these flags are present,
driver will fail Tx preparation or behave abnormally.

For example, ixgbe driver will refuse to process the packet with
only RTE_MBUF_F_TX_TUNNEL_GENEVE and RTE_MBUF_F_TX_OUTER_IPV4 set.
This pretty much breaks Geneve tunnels on these cards.

An extra check is added to make sure we don't have any unexpected
Tx offload flags set.

Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
Reported-at: https://github.com/openvswitch/ovs-issues/issues/321
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 lib/netdev-dpdk.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

Comments

Mike Pattrick March 12, 2024, 1:36 p.m. UTC | #1
On Mon, Mar 11, 2024 at 2:31 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> Some drivers (primarily, Intel ones) do not expect any marking flags
> being set if no offloads are requested.  If these flags are present,
> driver will fail Tx preparation or behave abnormally.
>
> For example, ixgbe driver will refuse to process the packet with
> only RTE_MBUF_F_TX_TUNNEL_GENEVE and RTE_MBUF_F_TX_OUTER_IPV4 set.
> This pretty much breaks Geneve tunnels on these cards.
>
> An extra check is added to make sure we don't have any unexpected
> Tx offload flags set.
>
> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/321
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

Acked-by: Mike Pattrick <mkp@redhat.com>
Ilya Maximets March 13, 2024, 4:09 p.m. UTC | #2
On 3/12/24 14:36, Mike Pattrick wrote:
> On Mon, Mar 11, 2024 at 2:31 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>>
>> Some drivers (primarily, Intel ones) do not expect any marking flags
>> being set if no offloads are requested.  If these flags are present,
>> driver will fail Tx preparation or behave abnormally.
>>
>> For example, ixgbe driver will refuse to process the packet with
>> only RTE_MBUF_F_TX_TUNNEL_GENEVE and RTE_MBUF_F_TX_OUTER_IPV4 set.
>> This pretty much breaks Geneve tunnels on these cards.
>>
>> An extra check is added to make sure we don't have any unexpected
>> Tx offload flags set.
>>
>> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
>> Reported-at: https://github.com/openvswitch/ovs-issues/issues/321
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> 
> Acked-by: Mike Pattrick <mkp@redhat.com>
> 

Thanks!  Applied and backported to 3.3.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 9444c53b1..8c52accff 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -607,6 +607,9 @@  int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
 struct ingress_policer *
 netdev_dpdk_get_ingress_policer(const struct netdev_dpdk *dev);
 
+static void netdev_dpdk_mbuf_dump(const char *prefix, const char *message,
+                                  const struct rte_mbuf *);
+
 static bool
 is_dpdk_class(const struct netdev_class *class)
 {
@@ -2569,9 +2572,29 @@  netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, struct rte_mbuf *mbuf)
     struct dp_packet *pkt = CONTAINER_OF(mbuf, struct dp_packet, mbuf);
     struct tcp_header *th;
 
-    if (!(mbuf->ol_flags & (RTE_MBUF_F_TX_IP_CKSUM | RTE_MBUF_F_TX_L4_MASK
-                            | RTE_MBUF_F_TX_TCP_SEG))) {
-        mbuf->ol_flags &= ~(RTE_MBUF_F_TX_IPV4 | RTE_MBUF_F_TX_IPV6);
+    const uint64_t all_requests = (RTE_MBUF_F_TX_IP_CKSUM |
+                                   RTE_MBUF_F_TX_L4_MASK  |
+                                   RTE_MBUF_F_TX_OUTER_IP_CKSUM  |
+                                   RTE_MBUF_F_TX_OUTER_UDP_CKSUM |
+                                   RTE_MBUF_F_TX_TCP_SEG);
+    const uint64_t all_marks = (RTE_MBUF_F_TX_IPV4 |
+                                RTE_MBUF_F_TX_IPV6 |
+                                RTE_MBUF_F_TX_OUTER_IPV4 |
+                                RTE_MBUF_F_TX_OUTER_IPV6 |
+                                RTE_MBUF_F_TX_TUNNEL_MASK);
+
+    if (!(mbuf->ol_flags & all_requests)) {
+        /* No offloads requested, no marks should be set. */
+        mbuf->ol_flags &= ~all_marks;
+
+        uint64_t unexpected = mbuf->ol_flags & RTE_MBUF_F_TX_OFFLOAD_MASK;
+        if (OVS_UNLIKELY(unexpected)) {
+            VLOG_WARN_RL(&rl, "%s: Unexpected Tx offload flags: %#"PRIx64,
+                         netdev_get_name(&dev->up), unexpected);
+            netdev_dpdk_mbuf_dump(netdev_get_name(&dev->up),
+                                  "Packet with unexpected ol_flags", mbuf);
+            return false;
+        }
         return true;
     }