Re: Restructuring inpcb/in6pcb

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

On Fri, Oct 28, 2022 at 2:53 PM Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
>
> 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.

It seems that all known issues are resolved now.

Thank you for your help!

  ozaki-r