On 7/10/26 9:08 AM, David Marchand wrote: > Define a helper capable of segmenting packets from a batch and returning > them in the input batch. > This will allow calling GSO in other places of OVS during packet > processing. > > Signed-off-by: David Marchand <[email protected]> > --- > Changes since RFC v1: > - let big batches flow to the netdev ->send() ops, > > --- > Documentation/topics/userspace-tso.rst | 4 +- > lib/dp-packet-gso.c | 66 +++++++++------- > lib/dp-packet-gso.h | 4 +- > lib/dp-packet.h | 16 ++++ > lib/netdev.c | 100 ++----------------------- > tests/dpif-netdev.at | 30 ++++++++ > 6 files changed, 95 insertions(+), 125 deletions(-) > > diff --git a/Documentation/topics/userspace-tso.rst > b/Documentation/topics/userspace-tso.rst > index 9f29e5ee3f..4a1f972c73 100644 > --- a/Documentation/topics/userspace-tso.rst > +++ b/Documentation/topics/userspace-tso.rst > @@ -117,7 +117,7 @@ This case is common for port not supporting TSO at all. > A coverage counter exists to reflect when the software fallback helper > is called.:: > > - $ ovs-appctl coverage/read-counter netdev_soft_seg_good > + $ ovs-appctl coverage/read-counter dp_packet_gso > 178760 > > Another possibility is when the `TSO` packet involves a header encapsulation > @@ -128,7 +128,7 @@ OvS does some partial segmentation based on the > segmentation size coming > from the receiving port. > A coverage counter exists to reflect when this optimisation is called.:: > > - $ ovs-appctl coverage/read-counter netdev_partial_seg_good > + $ ovs-appctl coverage/read-counter dp_packet_partial_gso > 1345 > > ~~~~~~~~~~~ > diff --git a/lib/dp-packet-gso.c b/lib/dp-packet-gso.c > index 22e8a6cd18..8c88173520 100644 > --- a/lib/dp-packet-gso.c > +++ b/lib/dp-packet-gso.c > @@ -18,6 +18,7 @@ > #include <stdlib.h> > #include <string.h> > > +#include "coverage.h" > #include "dp-packet.h" > #include "dp-packet-gso.h" > #include "netdev-provider.h" > @@ -25,6 +26,9 @@ > > VLOG_DEFINE_THIS_MODULE(dp_packet_gso); > > +COVERAGE_DEFINE(dp_packet_gso); > +COVERAGE_DEFINE(dp_packet_partial_gso); > + > /* Retuns a new packet that is a segment of packet 'p'. > * > * The new packet is initialized with 'hdr_len' bytes from the > @@ -176,10 +180,9 @@ dp_packet_gso_update_segment(struct dp_packet *seg, > unsigned int seg_no, > } > > static void > -dp_packet_gso__(struct dp_packet *p, struct dp_packet_batch **batches, > +dp_packet_gso__(struct dp_packet *p, struct dp_packet_batch *batch, > bool partial_seg) > { > - struct dp_packet_batch *curr_batch = *batches; > struct dp_packet *seg; > unsigned int n_segs; > uint16_t tso_segsz; > @@ -196,13 +199,10 @@ dp_packet_gso__(struct dp_packet *p, struct > dp_packet_batch **batches, > > /* Put back the first segment in the batch, it will be trimmed after > * all segments have been copied. */ > - if (dp_packet_batch_size(curr_batch) == NETDEV_MAX_BURST) { > - curr_batch++; > - } > - dp_packet_batch_add(curr_batch, p); > + dp_packet_batch_add(batch, p); > > if (n_segs <= 1) { > - goto out; > + return; > } > > if (dp_packet_tunnel(p)) { > @@ -227,11 +227,7 @@ dp_packet_gso__(struct dp_packet *p, struct > dp_packet_batch **batches, > tso_segsz); > dp_packet_gso_update_segment(seg, i, n_segs, tso_segsz, udp_tnl, > gre_tnl); > - > - if (dp_packet_batch_size(curr_batch) == NETDEV_MAX_BURST) { > - curr_batch++; > - } > - dp_packet_batch_add(curr_batch, seg); > + dp_packet_batch_add(batch, seg); > } > > last_seg: > @@ -240,11 +236,7 @@ last_seg: > data_len - (n_segs - 1) * tso_segsz); > dp_packet_gso_update_segment(seg, n_segs - 1, n_segs, tso_segsz, udp_tnl, > gre_tnl); > - > - if (dp_packet_batch_size(curr_batch) == NETDEV_MAX_BURST) { > - curr_batch++; > - } > - dp_packet_batch_add(curr_batch, seg); > + dp_packet_batch_add(batch, seg); > > first_seg: > if (partial_seg) { > @@ -261,19 +253,39 @@ first_seg: > dp_packet_set_tso_segsz(p, 0); > } > dp_packet_gso_update_segment(p, 0, n_segs, tso_segsz, udp_tnl, gre_tnl); > +} > > -out: > - *batches = curr_batch; > +static void > +dp_packet_gso_batch__(struct dp_packet_batch *batch, bool partial_seg) > +{ > + struct dp_packet_batch gso_batch; > + struct dp_packet *packet; > + > + dp_packet_batch_init(&gso_batch); > + gso_batch.trunc = batch->trunc; > + > + DP_PACKET_BATCH_FOR_EACH (k, packet, batch) { > + if (dp_packet_get_tso_segsz(packet)) { > + dp_packet_gso__(packet, &gso_batch, partial_seg); > + if (partial_seg) { > + COVERAGE_INC(dp_packet_partial_gso); > + } else { > + COVERAGE_INC(dp_packet_gso); > + } > + } else { > + dp_packet_batch_add(&gso_batch, packet); > + } > + } > + > + dp_packet_batch_swap(batch, &gso_batch); > + dp_packet_batch_destroy(&gso_batch); > } > > -/* Perform software segmentation on packet 'p'. > - * > - * Segments packet 'p' into the array of preallocated batches in 'batches', > - * updating the 'batches' pointer as needed. */ > +/* Segment GSO packets and send them back to caller in the input batch. */ > void > -dp_packet_gso(struct dp_packet *p, struct dp_packet_batch **batches) > +dp_packet_gso_batch(struct dp_packet_batch *b) > { > - dp_packet_gso__(p, batches, false); > + dp_packet_gso_batch__(b, false); > } > > /* Perform partial software segmentation on packet 'p'. > @@ -283,7 +295,7 @@ dp_packet_gso(struct dp_packet *p, struct dp_packet_batch > **batches) > * of preallocated batches in 'batches', updating the 'batches' pointer > * as needed. */ > void > -dp_packet_gso_partial(struct dp_packet *p, struct dp_packet_batch **batches) > +dp_packet_gso_batch_partial(struct dp_packet_batch *b) > { > - dp_packet_gso__(p, batches, true); > + dp_packet_gso_batch__(b, true); > } > diff --git a/lib/dp-packet-gso.h b/lib/dp-packet-gso.h > index 8d16179026..3549dcf4a3 100644 > --- a/lib/dp-packet-gso.h > +++ b/lib/dp-packet-gso.h > @@ -17,9 +17,9 @@ > #ifndef DP_PACKET_GSO_H > #define DP_PACKET_GSO_H 1 > > -void dp_packet_gso(struct dp_packet *, struct dp_packet_batch **); > +void dp_packet_gso_batch(struct dp_packet_batch *); > unsigned int dp_packet_gso_nr_segs(struct dp_packet *); > -void dp_packet_gso_partial(struct dp_packet *, struct dp_packet_batch **); > +void dp_packet_gso_batch_partial(struct dp_packet_batch *); > unsigned int dp_packet_gso_partial_nr_segs(struct dp_packet *); > > #endif /* dp-packet-gso.h */ > diff --git a/lib/dp-packet.h b/lib/dp-packet.h > index fbb2320b3c..5013654606 100644 > --- a/lib/dp-packet.h > +++ b/lib/dp-packet.h > @@ -978,6 +978,22 @@ dp_packet_batch_clone(struct dp_packet_batch *dst, > dst->trunc = src->trunc; > } > > +/* Swaps content of two batches. */ > +static inline void > +dp_packet_batch_swap(struct dp_packet_batch *a, struct dp_packet_batch *b) > +{ > + struct dp_packet_batch c = *a; > + > + *a = *b; > + *b = c; > + if (a->packets == b->embedded) { > + a->packets = a->embedded; > + } > + if (b->packets == a->embedded) { > + b->packets = b->embedded; > + } > +} > + > static inline void > dp_packet_batch_destroy(struct dp_packet_batch *batch) > { > diff --git a/lib/netdev.c b/lib/netdev.c > index 38c83e7395..330c462e98 100644 > --- a/lib/netdev.c > +++ b/lib/netdev.c > @@ -66,8 +66,6 @@ COVERAGE_DEFINE(netdev_sent); > COVERAGE_DEFINE(netdev_add_router); > COVERAGE_DEFINE(netdev_get_stats); > COVERAGE_DEFINE(netdev_push_header_drops); > -COVERAGE_DEFINE(netdev_soft_seg_good); > -COVERAGE_DEFINE(netdev_partial_seg_good); > > struct netdev_saved_flags { > struct netdev *netdev; > @@ -782,92 +780,6 @@ netdev_get_pt_mode(const struct netdev *netdev) > : NETDEV_PT_LEGACY_L2); > } > > -/* Attempts to segment GSO flagged packets and send them as multiple bundles. > - * This function is only used if at least one packet in the current batch is > - * flagged for TSO and the netdev does not support this. > - * > - * The return value is 0 if all batches sent successfully, and an error code > - * from netdev_class->send() if at least one batch failed to send. */ > -static int > -netdev_send_tso(struct netdev *netdev, int qid, > - struct dp_packet_batch *batch, bool concurrent_txq, > - bool partial_seg) > -{ > - struct dp_packet_batch *batches; > - struct dp_packet *packet; > - int retval = 0; > - int n_packets; > - int n_batches; > - int error; > - > - /* Calculate the total number of packets in the batch after > - * the (partial?) segmentation. */ > - n_packets = 0; > - DP_PACKET_BATCH_FOR_EACH (i, packet, batch) { > - if (dp_packet_get_tso_segsz(packet)) { > - if (partial_seg) { > - n_packets += dp_packet_gso_partial_nr_segs(packet); > - } else { > - n_packets += dp_packet_gso_nr_segs(packet); > - } > - } else { > - n_packets++; > - } > - } > - > - if (!n_packets) { > - return 0; > - } > - > - /* Allocate enough batches to store all the packets in order. */ > - n_batches = DIV_ROUND_UP(n_packets, NETDEV_MAX_BURST); > - batches = xmalloc(n_batches * sizeof *batches); > - > - struct dp_packet_batch *curr_batch = batches; > - struct dp_packet_batch *last_batch = &batches[n_batches - 1]; > - for (curr_batch = batches; curr_batch <= last_batch; curr_batch++) { > - dp_packet_batch_init(curr_batch); > - } > - > - /* Do the packet segmentation if TSO is flagged. */ > - size_t size = dp_packet_batch_size(batch); > - size_t k; > - curr_batch = batches; > - DP_PACKET_BATCH_REFILL_FOR_EACH (k, size, packet, batch) { > - if (dp_packet_get_tso_segsz(packet)) { > - if (partial_seg) { > - dp_packet_gso_partial(packet, &curr_batch); > - COVERAGE_INC(netdev_partial_seg_good); > - } else { > - dp_packet_gso(packet, &curr_batch); > - COVERAGE_INC(netdev_soft_seg_good); > - } > - } else { > - if (dp_packet_batch_size(curr_batch) == NETDEV_MAX_BURST) { > - curr_batch++; > - } > - dp_packet_batch_add(curr_batch, packet); > - } > - } > - > - for (curr_batch = batches; curr_batch <= last_batch; curr_batch++) { > - DP_PACKET_BATCH_FOR_EACH (i, packet, curr_batch) { > - dp_packet_ol_send_prepare(packet, netdev->ol_flags); > - } > - > - error = netdev->netdev_class->send(netdev, qid, curr_batch, > - concurrent_txq); > - if (!error) { > - COVERAGE_INC(netdev_sent); > - } else { > - retval = error; > - } > - dp_packet_batch_destroy(curr_batch); > - } > - free(batches); > - return retval; > -} > - > /* Sends 'batch' on 'netdev'. Returns 0 if successful (for every packet), > * otherwise a positive errno value. Returns EAGAIN without blocking if > * at least one the packets cannot be queued immediately. Returns EMSGSIZE > @@ -905,8 +817,8 @@ netdev_send(struct netdev *netdev, int qid, struct > dp_packet_batch *batch, > if (!(netdev_flags & NETDEV_TX_OFFLOAD_TCP_TSO)) { > DP_PACKET_BATCH_FOR_EACH (i, packet, batch) { > if (dp_packet_get_tso_segsz(packet)) { > - return netdev_send_tso(netdev, qid, batch, > concurrent_txq, > - false); > + dp_packet_gso_batch(batch); > + break; > } > } > } else if (!(netdev_flags & (NETDEV_TX_VXLAN_TNL_TSO | > @@ -915,16 +827,16 @@ netdev_send(struct netdev *netdev, int qid, struct > dp_packet_batch *batch, > DP_PACKET_BATCH_FOR_EACH (i, packet, batch) { > if (dp_packet_get_tso_segsz(packet) > && dp_packet_tunnel(packet)) { > - return netdev_send_tso(netdev, qid, batch, > concurrent_txq, > - false); > + dp_packet_gso_batch(batch); > + break; > } > } > } else if (!(netdev_flags & NETDEV_TX_OFFLOAD_OUTER_UDP_CKSUM)) { > DP_PACKET_BATCH_FOR_EACH (i, packet, batch) { > if (dp_packet_get_tso_segsz(packet) > && dp_packet_gso_partial_nr_segs(packet) != 1) { > - return netdev_send_tso(netdev, qid, batch, > concurrent_txq, > - true); > + dp_packet_gso_batch_partial(batch); > + break; > } > } > } > diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at > index fec2c60311..71cce9f895 100644 > --- a/tests/dpif-netdev.at > +++ b/tests/dpif-netdev.at > @@ -3633,6 +3633,36 @@ AT_CHECK([ovs-pcap p3-tx.pcap | wc -l], [0], [2 > OVS_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([dpif-netdev - tso + dynamic packet batch grow]) > +AT_KEYWORDS([userspace offload]) > +OVS_VSWITCHD_START( > + [set Open_vSwitch . other_config:userspace-tso-enable=true]) > +add_of_ports --pcap br0 $(seq 1 2) > + > +AT_CHECK([ovs-vsctl set Interface p1 options:ol_tso_segsz=1000]) > + > +AT_CHECK([ovs-ofctl add-flow br0 in_port=1,actions=output:2]) > + > +OVS_CHECK_BATCH_GROW([0]) > +CHECK_COVERAGE([dp_packet_gso], [0]) > + > +flow_s="in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:01:00),eth_type(0x0800), > \ > + ipv4(src=10.0.1.1,dst=10.0.0.1,proto=6,tos=0,ttl=64,frag=no), \ > + tcp(src=1234,dst=80),tcp_flags(ack)" > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 "${flow_s}" --len 35000]) > + > +dnl Interface statistics and coverage counters take some time to refresh. > +OVS_WAIT_UNTIL([test $(ovs-vsctl get interface p1 statistics:rx_packets) -eq > 1])
nit: AT_CHECK([ovs-vsctl wait-until interface p1 statistics:rx_packets=1]) > +CHECK_COVERAGE([netdev_received], [1]) > +OVS_CHECK_BATCH_GROW([1]) > +CHECK_COVERAGE([dp_packet_gso], [1]) > +CHECK_COVERAGE([netdev_sent], [1]) > +AT_CHECK([ovs-vsctl get interface p2 statistics:tx_packets], [0], [35 > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([dpif-netdev - revalidators handle dp modification fail correctly]) > OVS_VSWITCHD_START( > [add-port br0 p1 \ _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
