On Mon, Jun 05, 2017 at 11:39:14AM +0200, Martin Pieuchot wrote:
> On 30/05/17(Tue) 14:05, Claudio Jeker wrote:
> > This is more or less the same thing for PF_KEY that we now do in PF_ROUTE.
> > Use one PCB LIST on the keycb and embedd the rawcb in that PF_KEY cb.
> > Diff also has a few variable renames in it to make this code less alien
> > regarding the rest of our kernel. Mainly use so instead of socket and
> > pfkeyv2_socket is also replaced with better variable names.
> >
> > This needs the previous diff I just sent out for PF_ROUTE.
> > After that I can make pfkey use the same SRPL_LIST as PF_ROUTE (from an
> > other diff) to unlock them more.
> > --
> > :wq Claudio
> >
> > Index: net/pfkeyv2.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> > retrieving revision 1.160
> > diff -u -p -r1.160 pfkeyv2.c
> > --- net/pfkeyv2.c 29 May 2017 20:31:12 -0000 1.160
> > +++ net/pfkeyv2.c 30 May 2017 08:44:05 -0000
> > @@ -212,71 +213,62 @@ pfkey_init(void)
> > int
> > pfkeyv2_attach(struct socket *so, int proto)
> > {
> > - struct pfkeyv2_socket *pfkeyv2_socket;
> > + struct rawcb *rp;
> > + struct keycb *pk;
> > int error;
> >
> > if ((so->so_state & SS_PRIV) == 0)
> > return EACCES;
> >
> > - if (!(so->so_pcb = malloc(sizeof(struct rawcb),
> > - M_PCB, M_DONTWAIT | M_ZERO)))
> > - return (ENOMEM);
> > -
> > - error = raw_attach(so, so->so_proto->pr_protocol);
> > - if (error)
> > - goto ret;
> > -
> > - ((struct rawcb *)so->so_pcb)->rcb_faddr = &pfkey_addr;
> > -
> > - if (!(pfkeyv2_socket = malloc(sizeof(struct pfkeyv2_socket),
> > - M_PFKEY, M_NOWAIT | M_ZERO)))
> > - return (ENOMEM);
> > + pk = malloc(sizeof(struct keycb), M_PCB, M_WAITOK | M_ZERO);
> > + rp = &pk->rcb;
> > + so->so_pcb = rp;
> > +
> > + error = raw_attach(so, proto);
> > + if (error) {
> > + free(pk, M_PCB, sizeof(struct keycb));
> > + return (error);
> > + }
> >
> > - LIST_INSERT_HEAD(&pfkeyv2_sockets, pfkeyv2_socket, kcb_list);
> > - pfkeyv2_socket->socket = so;
> > - pfkeyv2_socket->pid = curproc->p_p->ps_pid;
> > + rp->rcb_faddr = &pfkey_addr;
> > + pk->pid = curproc->p_p->ps_pid;
> >
> > /*
> > * XXX we should get this from the socket instead but
> > * XXX rawcb doesn't store the rdomain like inpcb does.
> > */
> > - pfkeyv2_socket->rdomain = rtable_l2(curproc->p_p->ps_rtableid);
> > + pk->rdomain = rtable_l2(curproc->p_p->ps_rtableid);
> > +
> > + LIST_INSERT_HEAD(&pfkeyv2_sockets, pk, kcb_list);
> >
> > so->so_options |= SO_USELOOPBACK;
> > soisconnected(so);
>
> Same comment as for PF_ROUTE, I'd do the insertion here.
>
See other mail. Will think about this a bit more.
> > return (0);
> > -ret:
> > - free(so->so_pcb, M_PCB, sizeof(struct rawcb));
> > - return (error);
> > }
> >
> > /*
> > * Close a PF_KEYv2 socket.
> > */
> > int
> > -pfkeyv2_detach(struct socket *socket, struct proc *p)
> > +pfkeyv2_detach(struct socket *so, struct proc *p)
> > {
> > - struct pfkeyv2_socket *pp;
> > + struct keycb *pp;
>
> I'd rename 'pp' into 'pk' for coherency since you're changing all the
> lines ;)
>
Done.
> > int error;
> >
> > - LIST_FOREACH(pp, &pfkeyv2_sockets, kcb_list)
> > - if (pp->socket == socket)
> > - break;
> > -
> > - if (pp) {
> > - LIST_REMOVE(pp, kcb_list);
> > + pp = sotokeycb(so);
> > + if (pp == NULL)
> > + return ENOTCONN;
> >
> > - if (pp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)
> > - nregistered--;
> > + LIST_REMOVE(pp, kcb_list);
> >
> > - if (pp->flags & PFKEYV2_SOCKETFLAGS_PROMISC)
> > - npromisc--;
> > + if (pp->flags & PFKEYV2_SOCKETFLAGS_REGISTERED)
> > + nregistered--;
> >
> > - free(pp, M_PFKEY, 0);
> > - }
> > + if (pp->flags & PFKEYV2_SOCKETFLAGS_PROMISC)
> > + npromisc--;
> >
> > - error = raw_usrreq(socket, PRU_DETACH, NULL, NULL, NULL, p);
> > + error = raw_usrreq(so, PRU_DETACH, NULL, NULL, NULL, p);
>
> Why not call raw_detach() directly and return 0?
>
One step at a time. Anyway. changed to code to do so now.
> > return (error);
> > }
>
> The rest is ok :)
>
--
:wq Claudio
