On Wed, Apr 3, 2024 at 8:13 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
> > - This patch fixes some misusage of the DPDK API.
>
> Hmm, I understand that the driver does something funny when it gets
> outer flags set without any inner flags, but how is that a misuse
> of the DPDK API?  Could you point me to the API docs that say that
> inner flags must always be set in this case or that setting only
> outer offloads is not allowed?

Setting the tunnel type (which is set along outer checksum in OVS) is
described as:

/**
 * Bits 45:48 used for the tunnel type.
 * The tunnel type must be specified for TSO or checksum on the inner part
 * of tunnel packets.
 * These flags can be used with RTE_MBUF_F_TX_TCP_SEG for TSO, or
 * RTE_MBUF_F_TX_xxx_CKSUM.
 * The mbuf fields for inner and outer header lengths are required:
 * outer_l2_len, outer_l3_len, l2_len, l3_len, l4_len and tso_segsz for TSO.
 */
#define RTE_MBUF_F_TX_TUNNEL_VXLAN   (0x1ULL << 45)
#define RTE_MBUF_F_TX_TUNNEL_GRE     (0x2ULL << 45)
#define RTE_MBUF_F_TX_TUNNEL_IPIP    (0x3ULL << 45)
#define RTE_MBUF_F_TX_TUNNEL_GENEVE  (0x4ULL << 45)
/** TX packet with MPLS-in-UDP RFC 7510 header. */
#define RTE_MBUF_F_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
#define RTE_MBUF_F_TX_TUNNEL_VXLAN_GPE (0x6ULL << 45)
#define RTE_MBUF_F_TX_TUNNEL_GTP       (0x7ULL << 45)
#define RTE_MBUF_F_TX_TUNNEL_ESP       (0x8ULL << 45)

It is not specified what to expect it neither TSO nor inner checksum
is requested.

In a same way, it is not described what to expect if outer API is
called with no inner offload.
Adding Ferruh and Thomas who may have one opinion.


>
> I agree that it seems safer to just downgrade all outer flags to
> inner ones on OVS side, when no inner offloads are requested, I'm
> just not sure I agree that it's an API misuse.  Especially since
> non-Intel cards seem to work fine.

I suppose you mean mlx5.
Has it been tested on other nics?


>
> > Basically, resolving a neighbor with ARP inside tunnels is broken on
> > Intel nics (even without re-enabling outer udp checksum).
> > This can be observed with the following debug patch at the end of
> > netdev_dpdk_prep_hwol_packet():
> >
> > +    char buf[256];
> > +    if (rte_get_tx_ol_flag_list(mbuf->ol_flags, buf, sizeof(buf)) != 0)
> > +        buf[0] = '\0';
> > +    VLOG_WARN("len=%u, ol_flags=%s, outer_l2_len=%u, outer_l3_len=%u,
> > l2_len=%u, l3_len=%u, l4_len=%u, tso_segsz=%u", mbuf->pkt_len, buf,
> > mbuf->outer_l2_len, mbuf->outer_l3_len, mbuf->l2_len, mbuf->l3_len,
> > mbuf->l4_len, mbuf->tso_segsz);
> >
> > Then doing a "arping" inside the tunnel triggers:
> > 2024-04-03T16:05:40.920Z|00014|netdev_dpdk(pmd-c03/id:8)|WARN|len=96,
> > ol_flags=RTE_MBUF_F_TX_L4_NO_CKSUM RTE_MBUF_F_TX_OUTER_IP_CKSUM
> > RTE_MBUF_F_TX_OUTER_IPV4 RTE_MBUF_F_TX_TUNNEL_VXLAN , outer_l2_len=18,
> > outer_l3_len=20, l2_len=0, l3_len=0, l4_len=0, tso_segsz=0
> > 2024-04-03T16:05:40.920Z|00012|dpdk|WARN|ice_interrupt_handler():
> > OICR: MDD event
> >
> > We need this fix in OVS regardless of the outer udp checksum issue.
> > I'll respin this fix in a new series, without touching UDP checksum capa.
> >
> >
> > - It does seem that X710 nics have no support for outer udp checksum
> > (according to its datasheet). Some X722 version may have support for
> > it, but net/i40e does not configure the Tx descriptor accordingly.
> > On the other hand, E810 ones seem fine (according to its datasheet).
> >
> > After more debugging, I managed to get outer udp checksum working.
> > I understand the DPDK rte_net_intel_cksum_flags_prepare() helper does
> > not set the pseudo header checksum in the outer udp header.
> > I proposed a fix in the dpdk bz.
> >
> > Waiting for the fix on DPDK side... it is still possible to add the
> > missing bits in OVS (see the branch I pointed at in the OVS issue).
>
> Since this feature never worked with ice in OVS and it is experimental,
> I tend to think that we should just disable it for ice as well until
> DPDK is fixed.
>
> A little too many fixes for that thing we have already and this one will
> involve some extra driver-specific logic that we don't have any automated
> tests for.

I don't mind waiting for the DPDK fix before re-enabling outer udp and
other offloads.


-- 
David Marchand

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to