Date: Fri, 5 Sep 2014 14:19:18 -0700 From: Eric Joyner <ricera10@gmail.com> To: Hans Petter Selasky <hps@selasky.org> Cc: "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>, FreeBSD Current <freebsd-current@freebsd.org>, Scott Long <scottl@freebsd.org>, Jack F Vogel <jfvogel@gmail.com> Subject: Re: [RFC] Patch to improve TSO limitation formula in general Message-ID: <CA%2Bb0zg9MbRFYbJL6hZ4-j6ChQ=LPSfCkLRcDvUSnUH%2Bug%2BY-zA@mail.gmail.com> In-Reply-To: <540A0301.9040701@selasky.org> References: <540A0301.9040701@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
There are some concerns if we use this with devices that ixl supports:
- The maximum fragment size is 16KB-1, which isn't a power of 2.
- You can't get the maximum TSO size for ixl devices by multiplying the
maximum number of fragments by the maximum size.
Instead the number of fragments is AFAIK unlimited, but a segment can only
span 8 mbufs (including the [up to 3] mbufs containing the header), and the
maximum TSO size is 256KB.
And one question:
- Is hdr_size_log2 supposed to be the length of the L2 header? We can fit
254 L2 bytes in our hardware during a TSO, so if that's the value, I guess
that's fine, barring the it-not-being-a-power-of-2 issue.
With all that said, the 8 mbuf limit per segment issue is a TSO limitation
that we'd like to notify the stack about, so I wonder if that could be
incorporated along with this. Right now, our driver checks to see if a
segment in a TSO spans more than six mbufs and then m_defrag()'s the entire
chain if one exists; it's less than optimal but necessary to prevent errors.
- Eric
---
- Eric Joyner
On Fri, Sep 5, 2014 at 11:37 AM, Hans Petter Selasky <hps@selasky.org>
wrote:
> Hi,
>
> I've tested the attached patch with success and would like to have some
> feedback from other FreeBSD network developers. The problem is that the
> current TSO limitation only limits the number of bytes that can be
> transferred in a TSO packet and not the number of mbuf's.
>
> The current solution is to have a quick and dirty custom m_dup() in the TX
> path to re-allocate the mbuf chains into 4K ones to make it simple. All of
> this hack can be avoided if the definition of the TSO limit can be changed
> a bit, like shown here:
>
>
> /*
> + * Structure defining hardware TSO limits.
> + */
> +struct if_tso_limit {
> + u_int raw_value[0]; /* access all fields as one */
> + u_char frag_count; /* maximum number of fragments: 1..255 */
> + u_char frag_size_log2; /* maximum fragment size: 2 ** (12..16) */
> + u_char hdr_size_log2; /* maximum header size: 2 ** (2..8) */
> + u_char reserved; /* zero */
> +};
>
>
> First we need to know the maximum fragment count. Typical value is 32.
> Second we need to know the maximum fragment size. Typical value is 4K.
> Last we need to know of any headers that should be subtracted from the
> maximum. Hence this code is running in the fast path, I would like to use
> "u_char" for all fields and allow copy-only access as a "u_int" as an
> optimization. This avoids cludges and messing with additional header files.
>
> I would like to push this patch after some more testing to -current and
> then to 10-stable hopefully before the coming 10-release, because the
> current solution is affecting performance of the Mellanox based network
> adapters in an unfair way. For example by setting the current TSO limit to
> 32KBytes which will be OK for all-2K fragments, we see a severe degradation
> in performance. Even though the hardware is fully capable of transmitting
> 16 4K mbufs.
>
> Comments and reviews are welcome!
>
> --HPS
>
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2Bb0zg9MbRFYbJL6hZ4-j6ChQ=LPSfCkLRcDvUSnUH%2Bug%2BY-zA>