Re: Restructuring inpcb/in6pcb

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Wed, Oct 26, 2022 at 5:36 PM Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
>
> On Wed, Oct 26, 2022 at 12:00 AM Christos Zoulas <christos%astron.com@localhost> wrote:
> >
> > In article <CAKrYomi=8oWz4=_Yd4hWvwP6xuExZmq+G89rdVdDP8+k-T-qwQ%mail.gmail.com@localhost>,
> > Ryota Ozaki  <ozaki.ryota%gmail.com@localhost> wrote:
> > >Hi,
> > >
> > >Data structures of network protocol control blocks (PCBs)
> > >(struct inpcb, in6pcb and inpcb_hdr) are not organized well.
> > >Users of the data structures have to handle them separately
> > >and thus the code becomes duplicated and cluttered.
> > >
> > >The proposal restructures the data structures, which eliminates
> > >duplicate and cluttered code and makes further changes easy.
> > >
> > >A typical cleanup by the change looks like this:
> > >
> > >-       if (tp->t_inpcb)
> > >-               so = tp->t_inpcb->inp_socket;
> > >-#ifdef INET6
> > >-       else if (tp->t_in6pcb)
> > >-               so = tp->t_in6pcb->in6p_socket;
> > >-#endif
> > >+       so = tp->t_inpcb->inp_socket;
> > >
> > >Also some duplicated functions for IPv4 and IPv6 are integrated:
> > >tcp6_notify, tcp6_quench, etc.
> > >
> > >The change consists of two parts.  The first half of the series of
> > >commits tries to integrate all the data structures into one structure
> > >(struct inpcb).  The commits cleans up ugly code like above.
> > >However, because the structure includes both IPv4 and IPv6 stuff,
> > >the data size for IPv4 increases by 40 bytes (from 248 to 288).
> > >
> > >The second half of the series of commits addresses the increasement
> > >of the data size by separating the data structure again while keeping
> > >the code simple.  By the change, struct inpcb has only common data
> > >and newly introduced data structures, struct in4pcb and struct in6pcb
> > >inherited from struct inpcb, have dedicated data for each protocol.
> > >Even after the separation, users don't need to recognize the separation
> > >and just need to use some macros to access dedicated data.
> > >For example, inp->inp_laddr, which represents the local IPv4 address,
> > >is now accessed through in4p_laddr(inp).  Using these macros adds
> > >some code complexity, but this is a trade-off between data size and
> > >code complexity.
> > >
> > >The diffs are here:
> > >- https://www.netbsd.org/~ozaki-r/restructure-inpcb-1.patch
> > >- https://www.netbsd.org/~ozaki-r/restructure-inpcb-2.patch
> > >
> > >Also, you can see individual commits at github:
> > >  https://github.com/ozaki-r/netbsd-src/commits/restructure-inpcb
> > >
> > >
> > >We can adopt either of the whole changes or only changes of the first half.
> > >Which should we choose? Smaller data size or simpler code?
> > >
> > >By the way, I think the changes should be committed (if permitted)
> > >after branching netbsd-10.  When will the netbsd-10 branch come?
> > >If it doesn't come soon, is it ok to commit the changes before branching?
> >
> > Thanks! I wanted to do this for a long time... I don't see why it should wait
> > for the branch...
>
> Just because it's relatively big, but if nobody cares I'm not going to
> care too :)

I've committed the changes anyway.  Let me know if something goes wrong.

  ozaki-r