On Mon, Jun 05, 2017 at 11:32:06AM +0200, Martin Pieuchot wrote:
> On 30/05/17(Tue) 13:59, Claudio Jeker wrote:
> > This is a step I need to do to make progress on the PF_KEY cleanup I'm
> > doing. Both PF_ROUTE and PF_KEY need to start to take care of their own
> > PCB list and so move the LIST_ENTRY out of rawcb into routecb.
> > This allows me to do the same in PF_KEY which will be sent as the next
> > diff.
> >
> > --
> > :wq Claudio
> >
> > Index: net/rtsock.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/rtsock.c,v
> > retrieving revision 1.237
> > diff -u -p -r1.237 rtsock.c
> > --- net/rtsock.c 19 Apr 2017 15:21:54 -0000 1.237
> > +++ net/rtsock.c 30 May 2017 10:29:25 -0000
> > @@ -154,45 +157,50 @@ struct route_cb route_cb;
> >
> > #define ROUTE_DESYNC_RESEND_TIMEOUT (hz / 5) /* In hz */
> >
> > +void
> > +route_prinit(void)
> > +{
> > + LIST_INIT(&route_cb.rcb);
> > +}
> > +
> > +
> > int
> > route_usrreq(struct socket *so, int req, struct mbuf *m, struct mbuf *nam,
> > struct mbuf *control, struct proc *p)
> > {
> > - struct rawcb *rp;
> > struct routecb *rop;
> > int af;
> > int error = 0;
> >
> > - rp = sotorawcb(so);
> > + rop = sotoroutecb(so);
> > + if (rop == NULL)
> > + return ENOTCONN;
>
> Previously raw_usrreq() was returning EINVAL in that case. Does it
> matter?
>
> You should also call m_freem(m), because even if PRU_RCVD and PRU_DETACH
> do not take any argument, we cannot be sure all other code paths cannot
> be reached. That's one of the reasons I'm suggesting we split the PRU
> switches in multiple functions.
>
What about we make this a KASSERT()? I think it is impossible to get there
with a NULL pointer for the pcb.
> > @@ -240,10 +248,11 @@ route_attach(struct socket *so, int prot
> > #endif
> > rp->rcb_faddr = &route_src;
> > route_cb.any_count++;
> > + LIST_INSERT_HEAD(&route_cb.rcb, rop, rcb_list);
> > soisconnected(so);
> > so->so_options |= SO_USELOOPBACK;
>
> I'd do the insertion last. It doesn't matter if all operations are
> serialized but it doesn't cost anything and who knows what's coming
> next ;)
Actually that is the reason why I did it there. Doing the socket setup
last to ensure everything is settled before making the socket writeable.
At least the soisconnected() call will move the state of the socket ahead
and therefore other callers may start talking to it. On the other hand
route_input() does a somewhat bad job at figuring out which PCBs are
available.
> Other than that I like this diff very much.
>
--
:wq Claudio
