Hello Mike, On Mon, Dec 2, 2024 at 4:05 PM Mike Pattrick <[email protected]> wrote: > > This patch extends the userspace datapaths support of tunnel tso from > only supporting VxLAN and Geneve to also supporting GRE tunnels. There > is also a software fallback for cases where the egress netdev does not > support this feature. > > Signed-off-by: Mike Pattrick <[email protected]>
A general question, is it possible/what is missing to get erspan tunnel support? > > --- > v2: > - Corrected logic in reset_tcp_seg > - Sequenced GRE tunnels will now drop sequencing on TSO. > - Documentation removed. > > Signed-off-by: Mike Pattrick <[email protected]> > --- > lib/dp-packet-gso.c | 28 +++++++--- > lib/dp-packet.c | 2 + > lib/dp-packet.h | 22 ++++++++ > lib/netdev-dpdk.c | 14 +++++ > lib/netdev-native-tnl.c | 32 +++++++++--- > lib/netdev-provider.h | 1 + > lib/netdev.c | 12 ++++- > tests/dpif-netdev.at | 85 +++++++++++++++++++++++++++---- > tests/system-traffic.at | 110 +++++++++++++++++++++++++++++++++++++++- > 9 files changed, 278 insertions(+), 28 deletions(-) netdev_dpdk_prep_hwol_packet() is missing an update to accept gre tunnel checksum/tso offloading. @@ -2646,6 +2646,7 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev, struct rte_mbuf *mbuf) const uint64_t tunnel_type = mbuf->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK; if (OVS_UNLIKELY(tunnel_type && tunnel_type != RTE_MBUF_F_TX_TUNNEL_GENEVE && + tunnel_type != RTE_MBUF_F_TX_TUNNEL_GRE && tunnel_type != RTE_MBUF_F_TX_TUNNEL_VXLAN)) { VLOG_WARN_RL(&rl, "%s: Unexpected tunnel type: %#"PRIx64, netdev_get_name(&dev->up), tunnel_type); With this hunk, I tested with CX6 Dx, X710 and E810 on a simple br-int (172.23.24.1) -> gre0 / br0 (172.23.23.1) -> dpdk0 plugged to another system with netdevX (172.23.23.2) -> gretapX (172.23.24.2) setup. Enabling tso in OVS gives at least a 4x boost with iperf. Enabling/disabling gre csum works too. I did not test ipv6 yet. After going though OVS sources, I suspect we are missing an update in dp_netdev_recirculate(). Maybe adding a dp_packet_hwol_is_tunnel() helper (that shares a list of tunnel flags with dp_packet_hwol_reset_tunnel()) would avoid missing considerations about any tunnel offloading request. And I also wonder if netdev_linux_prepend_vnet_hdr() needs some update. [snip] > diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c > index 3e609cf54..9864a2a4b 100644 > --- a/lib/netdev-native-tnl.c > +++ b/lib/netdev-native-tnl.c > @@ -174,7 +174,8 @@ netdev_tnl_push_ip_header(struct dp_packet *packet, const > void *header, > packet->l4_ofs = dp_packet_size(packet) - *ip_tot_size; > > if (dp_packet_hwol_is_tunnel_geneve(packet) || > - dp_packet_hwol_is_tunnel_vxlan(packet)) { > + dp_packet_hwol_is_tunnel_vxlan(packet) || > + dp_packet_hwol_is_tunnel_gre(packet)) { > dp_packet_hwol_set_tx_outer_ipv6(packet); > } else { > dp_packet_hwol_set_tx_ipv6(packet); > @@ -187,7 +188,8 @@ netdev_tnl_push_ip_header(struct dp_packet *packet, const > void *header, > ip->ip_tot_len = htons(*ip_tot_size); > /* Postpone checksum to when the packet is pushed to the port. */ > if (dp_packet_hwol_is_tunnel_geneve(packet) || > - dp_packet_hwol_is_tunnel_vxlan(packet)) { > + dp_packet_hwol_is_tunnel_vxlan(packet) || > + dp_packet_hwol_is_tunnel_gre(packet)) { > dp_packet_hwol_set_tx_outer_ipv4(packet); > dp_packet_hwol_set_tx_outer_ipv4_csum(packet); > } else { > @@ -510,9 +512,13 @@ netdev_gre_push_header(const struct netdev *netdev, > const struct ovs_action_push_tnl *data) > { > struct netdev_vport *dev = netdev_vport_cast(netdev); > + uint16_t l3_ofs = packet->l3_ofs; > + uint16_t l4_ofs = packet->l4_ofs; > struct gre_base_hdr *greh; > int ip_tot_size; > > + dp_packet_hwol_set_tunnel_gre(packet); > + Why not call dp_packet_tnl_ol_process() here? (that would require updating this helper to handle gre tunnel type) > greh = netdev_tnl_push_ip_header(packet, data->header, data->header_len, > &ip_tot_size, 0); > > @@ -522,11 +528,23 @@ netdev_gre_push_header(const struct netdev *netdev, > } > > if (greh->flags & htons(GRE_SEQ)) { > - /* Last 4 byte is GRE seqno */ > - int seq_ofs = gre_header_len(greh->flags) - 4; > - ovs_16aligned_be32 *seq_opt = > - ALIGNED_CAST(ovs_16aligned_be32 *, (char *)greh + seq_ofs); > - put_16aligned_be32(seq_opt, > htonl(atomic_count_inc(&dev->gre_seqno))); > + if (!dp_packet_hwol_is_tso(packet)) { > + /* Last 4 byte is GRE seqno */ > + int seq_ofs = gre_header_len(greh->flags) - 4; > + ovs_16aligned_be32 *seq_opt = > + ALIGNED_CAST(ovs_16aligned_be32 *, (char *) greh + seq_ofs); > + put_16aligned_be32(seq_opt, > + htonl(atomic_count_inc(&dev->gre_seqno))); > + } else { > + VLOG_WARN_RL(&err_rl, "Cannot use GRE Sequence numbers with > TSO."); > + } > + } > + > + if (l3_ofs != UINT16_MAX) { > + packet->inner_l3_ofs = l3_ofs + data->header_len; > + } > + if (l4_ofs != UINT16_MAX) { > + packet->inner_l4_ofs = l4_ofs + data->header_len; > } > } > -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
