mbuf / lro: move defines from mbuf.h to tcp_lro.h

[bz]

Summary

The defines in mbuf.h lro_tcp_d_len, lro_tcp_d_csum, and lro_tcp_h_off
are private to tcp_lro.c and lro_etype is tied to
M_LRO_EHDRSTRP (M_PROTO6) already defined in tcp_lro.h.
They are all "Layer specific non-persistent local storage" as mbuf.h
calls the fields, so move them over to tcp_lro.h where they belong.

MFC after: 8 weeks

[afedorov]

sys/netinet/tcp_input.c

109 ↗

(On Diff #102837)

This #include is not used.

[bz]

Remove include of tcp_lro until nsegs is handled at another time.

[glebius]

I'm a bit on fence with this particular fields and with mbuf features in general. On one side keeping local flags/vars private to their modules is correct. On the other side, mbuf is the most shared structure in the network stack and having a whole picture of its use just looking at the structure definition is very valuable to understand that. Also, these preprocessor defines make debugging sessions hard. So the other way to go would be to eliminate PH_loc and defines and continue further unionization of the mbuf. Right now we have some kind of a mix of these two approaches.

[bz]

In D34298#776675, @glebius wrote:

I'm a bit on fence with this particular fields and with mbuf features in general. On one side keeping local flags/vars private to their modules is correct. On the other side, mbuf is the most shared structure in the network stack and having a whole picture of its use just looking at the structure definition is very valuable to understand that. Also, these preprocessor defines make debugging sessions hard. So the other way to go would be to eliminate PH_loc and defines and continue further unionization of the mbuf. Right now we have some kind of a mix of these two approaches.

Historically all the per-layer bits were defined in the per-layer parts; firewalls had their bits, netinet6 theirs, wifi its, ..

It's only been lately that everything migrated into mbuf.h and to me that makes mbuf.h harder to read because there is a lot of non-mbuf-essential clutter in there.

Pulling in all fields into unions and pulling in all mtags and MPROTO defines (as the centralised solution) I believe would make this even worse.

Imagine putting a field in the mbuf union for a change like PH_loc.ptr from 3917c9ba653d804dc4f60b969ef5eee32525e5a6; that's overkill.

Right now you have a very general name in a union, and very specific names in the per-layer usage. I really don't need all these #defines in all files which include mbuf.h (which is actually dangerous as they may violate the "per layer" contraint a lot more easily that way) and especially not the descriptions of what they mean (which are entirely missing; last time I tried to figure out what tcp_tun_port was for I had to go and blame and then lookup a commit message because ...)

I believed the point of having an "anonymous" per layer storage in mbufs was exactly so one doesn't have to know what others are using it for ...

[glebius]

The rtwn example is of course overkill :)

I believed the point of having an "anonymous" per layer storage in mbufs was exactly so one doesn't have to know what others are using it for ...

The downside is that when you look at mbuf, you can't answer a question what is used and what is not. Which field can be shrink and which can't.

P.S. I'm not against your change, just having a discussion :)

[bz]

Yes, I understand. ... or which field is ab-used. Or which of these defines are stale ... or actually needed... or just obfuscate other code to make it 4 characters shorter (or longer in some cases)...

[bz]

Seems people have different plans for mbuf fields.