iflib: simplify lro & use lro_queue_mbuf()

[gallatin]

Summary

Simplify LRO handling in iflib by:

• removing forwarding checks ... LRO already does this
• use the rss assisted LRO (eg, tcp_lro_queue_mbuf()) interface, which allows LRO to accumulate packets and sort by RSS hash and arrival time. This leads to far better LRO aggregation rates on internet facing servers with lots of active connections.
• eliminate the cache-busting loop over chained packets

@olivier ran his forwarding tests and got the following results which indicate there is no statistically significant performance regression. To be honest, I'm a bit surprised that with LRO disabled there wasn't a major improvement.

IPv4:

x fbsd15-n301802, LRO enabled: inet4 pps forwarded
+ fbsd15-n301802, LRO disabled: inet4 pps forwarded
* fbsd15-n301802 with iflibLRO patch, LRO enabled: inet4 pps forwarded
% fbsd15-n301802 with iflibLRO patch, LRO disabled: inet4 pps forwarded
+--------------------------------------------------------------------------+
|+                       *  ** **                    x xO% ++  *x%% %      |
|                                                    |__M_A____|           |
|                     |_________________________A__________M______________||
|                         |__A__|                                          |
|                                                        |____A__M__|      |
+--------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   5     4435675.5       4545031     4473361.5     4490520.1     49385.446
+   5     3935135.5     4531915.5     4497616.5     4388806.2     254645.91
No difference proven at 95.0% confidence
*   5     4169932.5     4235808.5       4207651     4206904.7     26595.109
Difference at 95.0% confidence
        -283615 +/- 57845.4
        -6.31587% +/- 1.22557%
        (Student's t, pooled s = 39662.5)
%   5       4469367       4587448       4552957     4531026.3     52275.245
No difference proven at 95.0% confidence

And V6:
x fbsd15-n301802, LRO enabled: inet6 pps forwarded
+ fbsd15-n301802, LRO disabled: inet6 pps forwarded
* fbsd15-n301802 with iflibLRO patch, LRO enabled: inet6 pps forwarded
% fbsd15-n301802 with iflibLRO patch, LRO disabled: inet6 pps forwarded
+--------------------------------------------------------------------------+
|  **  *          *                    %     + *    x x% #    @   # +     +|
|                                               |_____MA______|            |
|                                             |____________A__M_________|  |
||__M__A______|                                                            |
|                                            |__________AM________|        |
+--------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   5     3892440.5       4006313       3935139     3941111.9     42360.416
+   5       3879069       4052423       3980431     3964857.8     76468.029
No difference proven at 95.0% confidence
*   5       3631117       3721512       3637506     3656538.3     37663.372
Difference at 95.0% confidence
        -284574 +/- 58455.5
        -7.22064% +/- 1.4244%
        (Student's t, pooled s = 40080.8)
%   5     3844229.5       4005560       3950130     3944272.7     61623.425
No difference proven at 95.0% confidence

[markj]

removing forwarding checks ... LRO already does this

But LRO only checks the forwarding configuration when it actually comes time to process packets. With this patch, if forwarding is enabled, we'll do a bunch of needless sorting.

FreeBSD/sys/net/iflib.c

2997 ↗

(On Diff #159908)

I can't tell what this comment means. Should we keep it?

[gallatin]

In D51772#1185535, @markj wrote:

removing forwarding checks ... LRO already does this

But LRO only checks the forwarding configuration when it actually comes time to process packets. With this patch, if forwarding is enabled, we'll do a bunch of needless sorting.

My opinion is "meh". With this patch, reasonable configs (LRO enabled w/o forwarding, LRO disabled with forwarding) are much faster in my testing, and the code is much simpler. So I think a minor pessimization of a silly case is acceptable for the simplicity and speedup in more common cases.

FreeBSD/sys/net/iflib.c

2997 ↗

(On Diff #159908)

No idea what it means either. It looks like new code, but it came from above. I'll get rid of it.

[markj]

In D51772#1185605, @gallatin wrote:

In D51772#1185535, @markj wrote:

removing forwarding checks ... LRO already does this

But LRO only checks the forwarding configuration when it actually comes time to process packets. With this patch, if forwarding is enabled, we'll do a bunch of needless sorting.

My opinion is "meh". With this patch, reasonable configs (LRO enabled w/o forwarding, LRO disabled with forwarding) are much faster in my testing, and the code is much simpler. So I think a minor pessimization of a silly case is acceptable for the simplicity and speedup in more common cases.

Ok, fair enough. I'd just suggest noting this in the commit log message.