Wang Zhan wrote:
> A GSO skb which exceeds an egress device limit loses its GSO feature mask
> and is segmented into individual packets. This is unnecessarily expensive
> when the device can still offload smaller TCP GSO skbs, which is easy to
> hit once one hop of a BIG TCP path raises gso_max_size and the next one
> does not.
>
> For an unencapsulated TCP GSO skb which exceeds gso_max_size or
> gso_max_segs, work out how many MSS segments each output skb may carry and
> resegment the skb with that max_segs bound instead. Encapsulated and
> frag-list skbs, GSO types the device cannot offload and bounds which leave
> room for a single MSS keep today's segmentation. The output obeys the GSO
> feature and limit contract the device already advertises, so it applies
> automatically, without extra device state or a userspace control.
>
> The output is a plain GSO skb, so its whole length lands in the 16-bit L3
> length field which inet_gso_segment() and ipv6_gso_segment() write. An
> egress limit above 64 KiB would give outputs whose length truncates, so
> the size limit is also capped at what that field can express.
>
> The helper runs on the skb which is handed to the driver, after
> validate_xmit_vlan() and sk_validate_xmit_skb(), and only from the
> netif_needs_gso() branch, so an skb which is not segmented pays nothing.
> An over-limit skb pays one device limit test and one ndo_features_check()
> for the bound, in exchange for keeping the output a GSO skb.
>
> Measured on a veth -> bridge -> TAP -> guest virtio-net path, with BIG TCP
> enabled on the veth endpoints and left off in the guest, so the skbs which
> the veth hop accepts have to be segmented before the TAP device. A single
> iperf3 TCP flow, six alternating runs per state (`-t 15 -O 5`, fixed CPU
> affinity and port tuple). The middle column is the same tree with the
> resegmentation disabled:
>
> protocol no BIG TCP mixed, no reseg mixed, resegmented
> TCP/IPv4 51.550 Gbps 15.850 Gbps 52.617 Gbps
> TCP/IPv6 52.050 Gbps 15.783 Gbps 51.933 Gbps
>
> Coefficient of variation for the two mixed columns was 0.48% and 0.82% for
> IPv4 and 0.44% and 0.44% for IPv6. A BIG TCP hop which feeds a 64 KiB hop
> loses 69% of the throughput of a path which never enables BIG TCP at all;
> bounded resegmentation recovers it, 3.3x over the existing segmentation
> path and within noise of the no BIG TCP baseline.
>
> Assisted-by: LLM
> Signed-off-by: Wang Zhan <[email protected]>
>
> ---
> v3:
> - the L3 protocol change moved to patch 1
> - fold skb_can_gso_resegment() into the helper, now skb_gso_output_max_segs()
> - pass the caller's features on unchanged, the bound only shapes the output
> - drop the scatter-gather and checksum tests skb_segment() applies itself
> - saturate the size limit with min(), drop the sub-MSS guard
> - cap the output size at GSO_LEGACY_MAX_SIZE, these outputs are not BIG TCP
> v2: https://lore.kernel.org/[email protected]/
> v1: https://lore.kernel.org/[email protected]/
> ---
> net/core/dev.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d66b667071837..728260772f349 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3933,6 +3933,63 @@ netdev_features_t netif_skb_features(struct sk_buff
> *skb)
> }
> EXPORT_SYMBOL(netif_skb_features);
>
> +static unsigned int
> +skb_gso_output_max_segs(struct sk_buff *skb, struct net_device *dev)
> +{
> + unsigned int mss = skb_shinfo(skb)->gso_size;
> + unsigned int gso_max_size, hdr_len, max_segs;
> + netdev_features_t features;
> + struct tcphdr _tcph, *th;
> +
> + /*
> + * The TCP frag-list path segments through skb_segment_list(), which
> + * does not carry max_segs, so bounded calls skip those skbs.
> + */
This comment answers only one of six conditions. And one that is
pretty straightforward. I'd drop.
In general, drop all too-obvious comments. AI has a habit of adding
a lot more, and more low information, comments than is customary in
kernel code (where we also have commit messages). Generally, repeating
what the code does is of little value.
> + if (!skb_is_gso(skb) || !skb_is_gso_tcp(skb) ||
> + skb->encapsulation || skb_has_frag_list(skb) ||
> + !skb_mac_header_was_set(skb) ||
> + !skb_transport_header_was_set(skb))
Conversely, they last two conditions are less obvious. Are they not
always true for a TSO packet?
> + return 0;
> +
> + th = skb_header_pointer(skb, skb_transport_offset(skb), sizeof(_tcph),
> + &_tcph);
> + if (!th || th->doff < sizeof(*th) / 4)
> + return 0;
> +
> + hdr_len = skb_transport_header(skb) - skb_mac_header(skb) +
> + th->doff * 4;
> +
> + /*
> + * The output stays a GSO skb, so the device has to offload the GSO
> + * type. The caller's features cannot tell that: this skb is over
> + * the device limits, so gso_features_check() has cleared their GSO
> + * bits. Compute the features again without the limit checks.
> + */
> + features = __netif_skb_features(skb, false);
> + if (!net_gso_ok(features | NETIF_F_GSO_ROBUST,
> + skb_shinfo(skb)->gso_type))
> + return 0;
> +
> + /*
> + * The output is a plain GSO skb, so its whole length lands in the
> + * 16-bit L3 length field: only a BIG TCP skb may exceed the legacy
> + * GSO size, and this path does not emit one.
> + */
> + gso_max_size = netif_get_gso_max_size(dev, vlan_get_protocol(skb));
Third time this is now called in validate_xmit_skb. Not sure if that can
easily be avoided.
> + gso_max_size = min(gso_max_size, GSO_LEGACY_MAX_SIZE);
> +
> + /*
> + * gso_within_dev_limits() accepts gso_segs == gso_max_segs but
> + * rejects skb->len >= gso_max_size, so only the size bound needs - 1.
> + * The min() keeps that subtraction from wrapping when the device
> + * limit is smaller than the headers.
> + */
> + max_segs = (gso_max_size - min(gso_max_size, hdr_len + 1)) / mss;
> + max_segs = min(max_segs, READ_ONCE(dev->gso_max_segs));
> +
> + return max_segs;
> +}
> +
> static int xmit_one(struct sk_buff *skb, struct net_device *dev,
> struct netdev_queue *txq, bool more)
> {
> @@ -4097,9 +4154,13 @@ static struct sk_buff *validate_xmit_skb(struct
> sk_buff *skb, struct net_device
> goto out_null;
>
> if (netif_needs_gso(skb, features)) {
> + unsigned int max_segs = 0;
> struct sk_buff *segs;
>
> - segs = skb_gso_segment(skb, features);
> + if (unlikely(!gso_within_dev_limits(skb, dev)))
> + max_segs = skb_gso_output_max_segs(skb, dev);
> +
> + segs = __skb_gso_segment(skb, features, true, max_segs);
> if (IS_ERR(segs)) {
> goto out_kfree_skb;
> } else if (segs) {
> --
> 2.47.3
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
