On Tue, Feb 28, 2017 at 01:28:45PM +0100, Martin Pieuchot wrote:
> Routing sockets do not really need the NET_LOCK(), only route_output()
> needs it.

That depends what the future use of the netlock will be.  route_input()
is called from tcp, icmp, arp timers.  Currently they have kernel
lock, but maybe we want to switch them to netlock only.

Do you see any problems when running this routing code with netlock?

> @@ -675,6 +678,7 @@ route_output(struct mbuf *m, ...)
>        * may be not consistent and could cause unexpected behaviour in other
>        * userland clients. Use goto fail instead.
>        */
> +     NET_LOCK(s);
>       switch (rtm->rtm_type) {
>       case RTM_ADD:
>               if (info.rti_info[RTAX_GATEWAY] == NULL) {

We may not sleep here.  The malloc(9) in route_output() uses M_NOWAIT,
so I guess the upper layer assumes that this code does not get
rescheduled.  One idea of netlock is to do the sleeps before the
atomic sections of the network code.

> @@ -943,20 +947,16 @@ change:
>       seq = rtm->rtm_seq;
>       free(rtm, M_RTABLE, 0);
>       rtm = rt_report(rt, type, seq, tableid);
> +flush:
> +     NET_UNLOCK(s);
> +     rtfree(rt);
>       if (rtm == NULL) {
>               error = ENOBUFS;
>               goto fail;
> -     }
> -
> -flush:
> -     if (rt)
> -             rtfree(rt);
> -     if (rtm) {
> -             if (error)
> -                     rtm->rtm_errno = error;
> -             else {
> -                     rtm->rtm_flags |= RTF_DONE;
> -             }
> +     } else if (error) {
> +             rtm->rtm_errno = error;
> +     } else {
> +             rtm->rtm_flags |= RTF_DONE;
>       }
>  
>       /*

Fixing the rt leak is OK bluhm@.  This should go in independently
so we can merge krw@'s RTM_PROPOSAL diff.

bluhm

Reply via email to