make ifconfig print vnetids in hex too

2016-12-12 Thread David Gwynne 
so things will look like this:

vlan9: flags=8002 mtu 1500
lladdr 00:00:00:00:00:00
index 11 priority 0 llprio 3
vlan: 70 parent interface: 
vnetid: 70 (0x46)
parent: none
groups: vlan
status: no carrier
vxlan0: flags=8802 mtu 1500
lladdr fe:e1:ba:d0:40:78
index 12 priority 0 llprio 3
vnetid: 4096 (0x1000)
groups: vxlan
media: Ethernet autoselect
status: active

some tools display "vnetids" as hex, so itd be nice to line them
up with ifconfig output a bit easier.

Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.334
diff -u -p -r1.334 ifconfig.c
--- sbin/ifconfig/ifconfig.c13 Dec 2016 01:36:21 -  1.334
+++ sbin/ifconfig/ifconfig.c13 Dec 2016 07:01:33 -
@@ -3664,7 +3664,7 @@ getvnetid(void)
return;
}
 
-   printf("\tvnetid: %lld\n", ifr.ifr_vnetid);
+   printf("\tvnetid: %lld (0x%llx)\n", ifr.ifr_vnetid, ifr.ifr_vnetid);
 }
 
 void

stop letting interface up/down fail silently

2016-12-12 Thread David Gwynne 
at the moment if ifconfig foo0 up fails, nothing says so because
the kernel doesnt propagate the return code from the IFFLAGS ioctl
back to userland.

it also reports a link state change before it actually tries to
bring the interface up or down.

the diff below shuffles the SIOCSIFFLAGS handling in if.c so itll
try to run the ioctl first, and only report link state changes if
IFF_UP changed.

ok?

Index: if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.464
diff -u -p -r1.464 if.c
--- if.c2 Dec 2016 18:32:38 -   1.464
+++ if.c13 Dec 2016 06:55:31 -
@@ -1776,20 +1776,26 @@ ifioctl(struct socket *so, u_long cmd, c
case SIOCSIFFLAGS:
if ((error = suser(p, 0)) != 0)
return (error);
-   if (ifp->if_flags & IFF_UP && (ifr->ifr_flags & IFF_UP) == 0) {
-   s = splnet();
-   if_down(ifp);
-   splx(s);
+
+   ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) |
+   (ifr->ifr_flags & ~IFF_CANTCHANGE);
+
+   if (ifp->if_ioctl != NULL) {
+   error = (*ifp->if_ioctl)(ifp, cmd, data);
+   if (error != 0) {
+   ifp->if_flags = oif_flags;
+   break;
+   }
}
-   if (ifr->ifr_flags & IFF_UP && (ifp->if_flags & IFF_UP) == 0) {
+
+   if (ISSET(oif_flags ^ ifp->if_flags, IFF_UP)) {
s = splnet();
-   if_up(ifp);
+   if (ISSET(ifp->if_flags, IFF_UP))
+   if_up(ifp);
+   else
+   if_down(ifp);
splx(s);
}
-   ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) |
-   (ifr->ifr_flags & ~IFF_CANTCHANGE);
-   if (ifp->if_ioctl)
-   (void) (*ifp->if_ioctl)(ifp, cmd, data);
break;
 
case SIOCSIFXFLAGS:

Re: dev/rnd.c more explicit_bzero

2016-12-12 Thread Theo de Raadt 
That's information is not a secret.

> Some functions in rnd have a timespec; make sure to zero it
> as already done with other buffers. Also do buf in
> dequeue_randomness().
> 
> - Michael
> 
> 
> Index: src/sys/dev/rnd.c
> ===
> RCS file: /cvs/src/sys/dev/rnd.c,v
> retrieving revision 1.191
> diff -u -p -u -r1.191 rnd.c
> --- src/sys/dev/rnd.c 8 Dec 2016 05:32:49 -   1.191
> +++ src/sys/dev/rnd.c 13 Dec 2016 04:49:24 -
> @@ -312,6 +312,7 @@ enqueue_randomness(u_int state, u_int va
>   timeout_add(_timeout, 1);
>  
>   mtx_leave();
> + explicit_bzero(, sizeof(ts));
>  }
>  
>  /*
> @@ -388,6 +389,7 @@ dequeue_randomness(void *v)
>   mtx_enter();
>   }
>   mtx_leave();
> + explicit_bzero(buf, sizeof(buf));
>  }
>  
>  /*
> @@ -458,6 +460,7 @@ suspend_randomness(void)
>   dequeue_randomness(NULL);
>   rs_count = 0;
>   arc4random_buf(entropy_pool, sizeof(entropy_pool));
> + explicit_bzero(, sizeof(ts));
>  }
>  
>  void
> @@ -473,6 +476,7 @@ resume_randomness(char *buf, size_t bufl
>  
>   dequeue_randomness(NULL);
>   rs_count = 0;
> + explicit_bzero(, sizeof(ts));
>  }
>  
>  static inline void _rs_rekey(u_char *dat, size_t datlen);
> @@ -523,6 +527,7 @@ _rs_stir(int do_lock)
>   mtx_leave();
>  
>   explicit_bzero(buf, sizeof(buf));
> + explicit_bzero(, sizeof(ts));
>  }
>  
>  static inline void
>

dev/rnd.c more explicit_bzero

2016-12-12 Thread Michael W. Bombardieri 
Hi,

Some functions in rnd have a timespec; make sure to zero it
as already done with other buffers. Also do buf in
dequeue_randomness().

- Michael


Index: src/sys/dev/rnd.c
===
RCS file: /cvs/src/sys/dev/rnd.c,v
retrieving revision 1.191
diff -u -p -u -r1.191 rnd.c
--- src/sys/dev/rnd.c   8 Dec 2016 05:32:49 -   1.191
+++ src/sys/dev/rnd.c   13 Dec 2016 04:49:24 -
@@ -312,6 +312,7 @@ enqueue_randomness(u_int state, u_int va
timeout_add(_timeout, 1);
 
mtx_leave();
+   explicit_bzero(, sizeof(ts));
 }
 
 /*
@@ -388,6 +389,7 @@ dequeue_randomness(void *v)
mtx_enter();
}
mtx_leave();
+   explicit_bzero(buf, sizeof(buf));
 }
 
 /*
@@ -458,6 +460,7 @@ suspend_randomness(void)
dequeue_randomness(NULL);
rs_count = 0;
arc4random_buf(entropy_pool, sizeof(entropy_pool));
+   explicit_bzero(, sizeof(ts));
 }
 
 void
@@ -473,6 +476,7 @@ resume_randomness(char *buf, size_t bufl
 
dequeue_randomness(NULL);
rs_count = 0;
+   explicit_bzero(, sizeof(ts));
 }
 
 static inline void _rs_rekey(u_char *dat, size_t datlen);
@@ -523,6 +527,7 @@ _rs_stir(int do_lock)
mtx_leave();
 
explicit_bzero(buf, sizeof(buf));
+   explicit_bzero(, sizeof(ts));
 }
 
 static inline void

Re: umb: aggregate packets on tx

2016-12-12 Thread Bryan Vyhmeister 
On Mon, Dec 12, 2016 at 02:50:50PM +0100, Gerhard Roth wrote:
> The current umb(4) implementation needs one USB transfer for every packet
> that is sent. With the following patch, we can now aggregate several
> packets from the ifq into one single USB transfer.
> 
> This may speed up the tx path. And even if it doesn't, at least it
> reduces the number of transfers required.

I applied this diff this morning and everything has been working
smoothly so far. There are no obvious regressions that I have noticed.
This is on an X1 Carbon (4th Gen) with an EM7455 just like the X260 has.

Bryan

Fix formatting of spamd.conf.5

2016-12-12 Thread Larry Hynes 
Add missing 'e's at EOL for the heise.de message.

Index: spamd.conf.5
===
RCS file: /cvs/src/share/man/man5/spamd.conf.5,v
retrieving revision 1.16
diff -u -p -r1.16 spamd.conf.5
--- spamd.conf.51 Jun 2016 21:57:03 -   1.16
+++ spamd.conf.512 Dec 2016 16:41:06 -
@@ -57,8 +57,8 @@ all:\e
 
 nixspam:\e
:black:\e
-   :msg="Your address %A is in the nixspam list\n\
-   See http://www.heise.de/ix/nixspam/dnsbl_en/ for details":\
+   :msg="Your address %A is in the nixspam list\n\e
+   See http://www.heise.de/ix/nixspam/dnsbl_en/ for details":\e
:method=http:\e
:file=www.openbsd.org/spamd/nixspam.gz

Re: multicast: propagate rdomain for add_vif

2016-12-12 Thread Martin Pieuchot 
On 12/12/16(Mon) 16:35, Mike Belopuhov wrote:
> On Mon, Dec 12, 2016 at 16:15 +0100, Martin Pieuchot wrote:
> > [...] 
> > Why pass the socket to add_vif() when you only need the rdomain?
> >
> 
> It makes add_vif take same arguments as some other functions called
> from ip_mrouter_set in that switch statement (like ip_mrouter_init).

Makes sense.

> > I'm also confused, if you need a rdomain shouldn't you be using
> > rtable_l2()?
> >
> 
> He has tested with rdomains, but in this particular case you're doing
> setsockopt, you don't know what rtable your socket is associated with
> and you're just passing the value along.  But besides, ifa_ifwithaddr
> will call the rtable_l2 itself (which makes sense I guess as we have
> to look up within the whole rdomain).

I just got confused by the description, it's indeed not rdomain related.

ok mpi@

Re: multicast: propagate rdomain for add_vif

2016-12-12 Thread Mike Belopuhov 
On Mon, Dec 12, 2016 at 16:15 +0100, Martin Pieuchot wrote:
> On 12/12/16(Mon) 16:09, Rafael Zalamena wrote:
> > After trying to run igmpproxy daemon in different rdomains I noted that
> > it fails with the following message:
> > "
> > ERRO: MRT_ADD_VIF; Errno(49): Can't assign requested address
> > "
> > 
> > In the following line:
> > "
> > if ( setsockopt( MRouterFD, IPPROTO_IP, MRT_ADD_VIF,
> >  (char *), sizeof( VifCtl ) ) )
> > my_log( LOG_ERR, errno, "MRT_ADD_VIF" );
> > "
> > 
> > With some help from mikeb@ we found out that even though the system is
> > configured for multicast (multicast=YES) it is failing to setsockopt(),
> > because it wasn't being able to add the address. We traced where was it
> > failing and found out that the MRT_ADD_VIF wasn't propagating the rdomain
> > so it was always trying to install in the rdomain 0 even when we ran
> > igmpproxy on a different domain.
> > 
> > This diff just makes ip_mrouter_set() propagate the rdomain so the
> > ifa_ifwithaddr() receives the right rdomain and not fail anymore.
> > 
> > ok?
> 
> Why pass the socket to add_vif() when you only need the rdomain?
>

It makes add_vif take same arguments as some other functions called
from ip_mrouter_set in that switch statement (like ip_mrouter_init).

> I'm also confused, if you need a rdomain shouldn't you be using
> rtable_l2()?
>

He has tested with rdomains, but in this particular case you're doing
setsockopt, you don't know what rtable your socket is associated with
and you're just passing the value along.  But besides, ifa_ifwithaddr
will call the rtable_l2 itself (which makes sense I guess as we have
to look up within the whole rdomain).

> > Index: sys/netinet/ip_mroute.c
> > ===
> > RCS file: /home/obsdcvs/src/sys/netinet/ip_mroute.c,v
> > retrieving revision 1.93
> > diff -u -p -r1.93 ip_mroute.c
> > --- sys/netinet/ip_mroute.c 29 Nov 2016 15:52:12 -  1.93
> > +++ sys/netinet/ip_mroute.c 12 Dec 2016 14:51:37 -
> > @@ -130,7 +130,7 @@ int get_vif_cnt(struct sioc_vif_req *);
> >  int get_vif_ctl(struct vifctl *);
> >  int ip_mrouter_init(struct socket *, struct mbuf *);
> >  int get_version(struct mbuf *);
> > -int add_vif(struct mbuf *);
> > +int add_vif(struct socket *, struct mbuf *);
> >  int del_vif(struct mbuf *);
> >  void update_mfc_params(struct mfc *, struct mfcctl2 *);
> >  void init_mfc_params(struct mfc *, struct mfcctl2 *);
> > @@ -293,7 +293,7 @@ ip_mrouter_set(struct socket *so, int op
> > error = ip_mrouter_done();
> > break;
> > case MRT_ADD_VIF:
> > -   error = add_vif(*mp);
> > +   error = add_vif(so, *mp);
> > break;
> > case MRT_DEL_VIF:
> > error = del_vif(*mp);
> > @@ -773,8 +773,9 @@ static struct sockaddr_in sin = { sizeof
> >   * Add a vif to the vif table
> >   */
> >  int
> > -add_vif(struct mbuf *m)
> > +add_vif(struct socket *so, struct mbuf *m)
> >  {
> > +   struct inpcb *inp;
> > struct vifctl *vifcp;
> > struct vif *vifp;
> > struct ifaddr *ifa;
> > @@ -809,8 +810,9 @@ add_vif(struct mbuf *m)
> > } else
> >  #endif
> > {
> > +   inp = sotoinpcb(so);
> > sin.sin_addr = vifcp->vifc_lcl_addr;
> > -   ifa = ifa_ifwithaddr(sintosa(), /* XXX */ 0);
> > +   ifa = ifa_ifwithaddr(sintosa(), inp->inp_rtableid);
> > if (ifa == NULL)
> > return (EADDRNOTAVAIL);
> > }
> > 
>

Re: [patch] turn igmpstat into a set of percpu counters

2016-12-12 Thread Martin Pieuchot 
On 11/12/16(Sun) 19:24, Dimitris Papastamos wrote:
> Hi,
> 
> I converted the igmp stats to use percpu counters.  This work is
> basically the same as what dlg@ did for other parts of the stack.
> I looked at the diff and adjusted it for igmp.

ok mpi@

> 
> diff --git a/sys/netinet/igmp.c b/sys/netinet/igmp.c
> index 11446ce4188..c2a0a4839b4 100644
> --- a/sys/netinet/igmp.c
> +++ b/sys/netinet/igmp.c
> @@ -101,13 +101,14 @@ int *igmpctl_vars[IGMPCTL_MAXID] = IGMPCTL_VARS;
>  int  igmp_timers_are_running;
>  static struct router_info *rti_head;
>  static struct mbuf *router_alert;
> -struct igmpstat igmpstat;
> +struct cpumem *igmpcounters;
>  
>  void igmp_checktimer(struct ifnet *);
>  void igmp_sendpkt(struct in_multi *, int, in_addr_t);
>  int rti_fill(struct in_multi *);
>  struct router_info * rti_find(struct ifnet *);
>  void igmp_input_if(struct ifnet *, struct mbuf *, int);
> +int igmp_sysctl_igmpstat(void *, size_t *, void *);
>  
>  void
>  igmp_init(void)
> @@ -117,6 +118,7 @@ igmp_init(void)
>   igmp_timers_are_running = 0;
>   rti_head = 0;
>  
> + igmpcounters = counters_alloc(igps_ncounters, M_COUNTERS);
>   router_alert = m_get(M_DONTWAIT, MT_DATA);
>   if (router_alert == NULL) {
>   printf("%s: no mbuf\n", __func__);
> @@ -217,7 +219,7 @@ igmp_input(struct mbuf *m, ...)
>   iphlen = va_arg(ap, int);
>   va_end(ap);
>  
> - ++igmpstat.igps_rcv_total;
> + igmpstat_inc(igps_rcv_total);
>  
>   ifp = if_get(m->m_pkthdr.ph_ifidx);
>   if (ifp == NULL) {
> @@ -248,14 +250,14 @@ igmp_input_if(struct ifnet *ifp, struct mbuf *m, int 
> iphlen)
>* Validate lengths
>*/
>   if (igmplen < IGMP_MINLEN) {
> - ++igmpstat.igps_rcv_tooshort;
> + igmpstat_inc(igps_rcv_tooshort);
>   m_freem(m);
>   return;
>   }
>   minlen = iphlen + IGMP_MINLEN;
>   if ((m->m_flags & M_EXT || m->m_len < minlen) &&
>   (m = m_pullup(m, minlen)) == NULL) {
> - ++igmpstat.igps_rcv_tooshort;
> + igmpstat_inc(igps_rcv_tooshort);
>   return;
>   }
>  
> @@ -266,7 +268,7 @@ igmp_input_if(struct ifnet *ifp, struct mbuf *m, int 
> iphlen)
>   m->m_len -= iphlen;
>   igmp = mtod(m, struct igmp *);
>   if (in_cksum(m, igmplen)) {
> - ++igmpstat.igps_rcv_badsum;
> + igmpstat_inc(igps_rcv_badsum);
>   m_freem(m);
>   return;
>   }
> @@ -277,7 +279,7 @@ igmp_input_if(struct ifnet *ifp, struct mbuf *m, int 
> iphlen)
>   switch (igmp->igmp_type) {
>  
>   case IGMP_HOST_MEMBERSHIP_QUERY:
> - ++igmpstat.igps_rcv_queries;
> + igmpstat_inc(igps_rcv_queries);
>  
>   if (ifp->if_flags & IFF_LOOPBACK)
>   break;
> @@ -292,7 +294,7 @@ igmp_input_if(struct ifnet *ifp, struct mbuf *m, int 
> iphlen)
>   rti->rti_age = 0;
>  
>   if (ip->ip_dst.s_addr != INADDR_ALLHOSTS_GROUP) {
> - ++igmpstat.igps_rcv_badqueries;
> + igmpstat_inc(igps_rcv_badqueries);
>   m_freem(m);
>   return;
>   }
> @@ -317,7 +319,7 @@ igmp_input_if(struct ifnet *ifp, struct mbuf *m, int 
> iphlen)
>   }
>   } else {
>   if (!IN_MULTICAST(ip->ip_dst.s_addr)) {
> - ++igmpstat.igps_rcv_badqueries;
> + igmpstat_inc(igps_rcv_badqueries);
>   m_freem(m);
>   return;
>   }
> @@ -367,14 +369,14 @@ igmp_input_if(struct ifnet *ifp, struct mbuf *m, int 
> iphlen)
>   break;
>  
>   case IGMP_v1_HOST_MEMBERSHIP_REPORT:
> - ++igmpstat.igps_rcv_reports;
> + igmpstat_inc(igps_rcv_reports);
>  
>   if (ifp->if_flags & IFF_LOOPBACK)
>   break;
>  
>   if (!IN_MULTICAST(igmp->igmp_group.s_addr) ||
>   igmp->igmp_group.s_addr != ip->ip_dst.s_addr) {
> - ++igmpstat.igps_rcv_badreports;
> + igmpstat_inc(igps_rcv_badreports);
>   m_freem(m);
>   return;
>   }
> @@ -401,7 +403,7 @@ igmp_input_if(struct ifnet *ifp, struct mbuf *m, int 
> iphlen)
>   IN_LOOKUP_MULTI(igmp->igmp_group, ifp, inm);
>   if (inm != NULL) {
>   inm->inm_timer = 0;
> - ++igmpstat.igps_rcv_ourreports;
> + igmpstat_inc(igps_rcv_ourreports);
>  
>   switch (inm->inm_state) {
>   case IGMP_IDLE_MEMBER:
> @@ -433,14 +435,14 @@ igmp_input_if(struct ifnet *ifp, struct mbuf *m, int 
> iphlen)
>   break;
>  #endif
>  
> -

Re: multicast: propagate rdomain for add_vif

2016-12-12 Thread Martin Pieuchot 
On 12/12/16(Mon) 16:09, Rafael Zalamena wrote:
> After trying to run igmpproxy daemon in different rdomains I noted that
> it fails with the following message:
> "
> ERRO: MRT_ADD_VIF; Errno(49): Can't assign requested address
> "
> 
> In the following line:
> "
> if ( setsockopt( MRouterFD, IPPROTO_IP, MRT_ADD_VIF,
>  (char *), sizeof( VifCtl ) ) )
> my_log( LOG_ERR, errno, "MRT_ADD_VIF" );
> "
> 
> With some help from mikeb@ we found out that even though the system is
> configured for multicast (multicast=YES) it is failing to setsockopt(),
> because it wasn't being able to add the address. We traced where was it
> failing and found out that the MRT_ADD_VIF wasn't propagating the rdomain
> so it was always trying to install in the rdomain 0 even when we ran
> igmpproxy on a different domain.
> 
> This diff just makes ip_mrouter_set() propagate the rdomain so the
> ifa_ifwithaddr() receives the right rdomain and not fail anymore.
> 
> ok?

Why pass the socket to add_vif() when you only need the rdomain?

I'm also confused, if you need a rdomain shouldn't you be using
rtable_l2()?

> Index: sys/netinet/ip_mroute.c
> ===
> RCS file: /home/obsdcvs/src/sys/netinet/ip_mroute.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 ip_mroute.c
> --- sys/netinet/ip_mroute.c   29 Nov 2016 15:52:12 -  1.93
> +++ sys/netinet/ip_mroute.c   12 Dec 2016 14:51:37 -
> @@ -130,7 +130,7 @@ int get_vif_cnt(struct sioc_vif_req *);
>  int get_vif_ctl(struct vifctl *);
>  int ip_mrouter_init(struct socket *, struct mbuf *);
>  int get_version(struct mbuf *);
> -int add_vif(struct mbuf *);
> +int add_vif(struct socket *, struct mbuf *);
>  int del_vif(struct mbuf *);
>  void update_mfc_params(struct mfc *, struct mfcctl2 *);
>  void init_mfc_params(struct mfc *, struct mfcctl2 *);
> @@ -293,7 +293,7 @@ ip_mrouter_set(struct socket *so, int op
>   error = ip_mrouter_done();
>   break;
>   case MRT_ADD_VIF:
> - error = add_vif(*mp);
> + error = add_vif(so, *mp);
>   break;
>   case MRT_DEL_VIF:
>   error = del_vif(*mp);
> @@ -773,8 +773,9 @@ static struct sockaddr_in sin = { sizeof
>   * Add a vif to the vif table
>   */
>  int
> -add_vif(struct mbuf *m)
> +add_vif(struct socket *so, struct mbuf *m)
>  {
> + struct inpcb *inp;
>   struct vifctl *vifcp;
>   struct vif *vifp;
>   struct ifaddr *ifa;
> @@ -809,8 +810,9 @@ add_vif(struct mbuf *m)
>   } else
>  #endif
>   {
> + inp = sotoinpcb(so);
>   sin.sin_addr = vifcp->vifc_lcl_addr;
> - ifa = ifa_ifwithaddr(sintosa(), /* XXX */ 0);
> + ifa = ifa_ifwithaddr(sintosa(), inp->inp_rtableid);
>   if (ifa == NULL)
>   return (EADDRNOTAVAIL);
>   }
>

multicast: propagate rdomain for add_vif

2016-12-12 Thread Rafael Zalamena 
After trying to run igmpproxy daemon in different rdomains I noted that
it fails with the following message:
"
ERRO: MRT_ADD_VIF; Errno(49): Can't assign requested address
"

In the following line:
"
if ( setsockopt( MRouterFD, IPPROTO_IP, MRT_ADD_VIF,
 (char *), sizeof( VifCtl ) ) )
my_log( LOG_ERR, errno, "MRT_ADD_VIF" );
"

With some help from mikeb@ we found out that even though the system is
configured for multicast (multicast=YES) it is failing to setsockopt(),
because it wasn't being able to add the address. We traced where was it
failing and found out that the MRT_ADD_VIF wasn't propagating the rdomain
so it was always trying to install in the rdomain 0 even when we ran
igmpproxy on a different domain.

This diff just makes ip_mrouter_set() propagate the rdomain so the
ifa_ifwithaddr() receives the right rdomain and not fail anymore.

ok?

Index: sys/netinet/ip_mroute.c
===
RCS file: /home/obsdcvs/src/sys/netinet/ip_mroute.c,v
retrieving revision 1.93
diff -u -p -r1.93 ip_mroute.c
--- sys/netinet/ip_mroute.c 29 Nov 2016 15:52:12 -  1.93
+++ sys/netinet/ip_mroute.c 12 Dec 2016 14:51:37 -
@@ -130,7 +130,7 @@ int get_vif_cnt(struct sioc_vif_req *);
 int get_vif_ctl(struct vifctl *);
 int ip_mrouter_init(struct socket *, struct mbuf *);
 int get_version(struct mbuf *);
-int add_vif(struct mbuf *);
+int add_vif(struct socket *, struct mbuf *);
 int del_vif(struct mbuf *);
 void update_mfc_params(struct mfc *, struct mfcctl2 *);
 void init_mfc_params(struct mfc *, struct mfcctl2 *);
@@ -293,7 +293,7 @@ ip_mrouter_set(struct socket *so, int op
error = ip_mrouter_done();
break;
case MRT_ADD_VIF:
-   error = add_vif(*mp);
+   error = add_vif(so, *mp);
break;
case MRT_DEL_VIF:
error = del_vif(*mp);
@@ -773,8 +773,9 @@ static struct sockaddr_in sin = { sizeof
  * Add a vif to the vif table
  */
 int
-add_vif(struct mbuf *m)
+add_vif(struct socket *so, struct mbuf *m)
 {
+   struct inpcb *inp;
struct vifctl *vifcp;
struct vif *vifp;
struct ifaddr *ifa;
@@ -809,8 +810,9 @@ add_vif(struct mbuf *m)
} else
 #endif
{
+   inp = sotoinpcb(so);
sin.sin_addr = vifcp->vifc_lcl_addr;
-   ifa = ifa_ifwithaddr(sintosa(), /* XXX */ 0);
+   ifa = ifa_ifwithaddr(sintosa(), inp->inp_rtableid);
if (ifa == NULL)
return (EADDRNOTAVAIL);
}

Re: bpf_mtap(9) w/o KERNEL_LOCK

2016-12-12 Thread Martin Pieuchot 
On 10/12/16(Sat) 23:01, Jonathan Matthew wrote:
> On Mon, Nov 28, 2016 at 04:01:14PM +0100, Martin Pieuchot wrote:
> > Last diff to trade the KERNEL_LOCK for a mutex in order to protect data
> > accessed inside bpf_catchpacket().
> > 
> > Note about the multiples data structures:
> > 
> >   - selwakeup() is called in a thread context (task) so we rely on the
> > KERNEL_LOCK() to serialize access to kqueue(9) data.
> > 
> >   - the global list of descriptor ``bpf_d_list``, accessed via 
> > bpfilter_lookup() is also protected by the KERNEL_LOCK().
> > 
> >   - bpf_get() increments the refcount of a descriptor.  It is needed
> > to increment it around ifppromisc() which can sleep.
> > 
> >   - A mutex now protects the rotating buffers and their associated
> > fields.
> >  
> >  - The dance around uiomove(9) is here to check that buffers aren't
> >   rotated while data is copied to userland.  Setting ``b->bd_fbuf''
> >   to NULL should be enough to let bpf_catchpacket() to drop the
> >   patcket.  But I added ``__in_uiomove'' to be able to have usable
> >   panic if something weird happen.
> > 
> > Comments, oks?
> 
> The thing I'm concerned about here is the call to bpf_wakeup() from
> bpf_catchpacket() - this modifies the bpf_d refcount outside the kernel lock.
> Elsewhere the refcount is modified while holding the kernel lock, with or
> without the mutex.  It'll never free the bpf_d there, since the bpf_d list
> still holds a reference, but the refcount operations aren't atomic, so the
> counter could get messed up.  Is there a reason not to use atomics there?

Good point.

Here's an updated diff that uses the atomics and addresses Alexander's
comments.


Index: net/bpf.c
===
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.155
diff -u -p -r1.155 bpf.c
--- net/bpf.c   28 Nov 2016 10:16:08 -  1.155
+++ net/bpf.c   12 Dec 2016 14:18:38 -
@@ -116,6 +116,9 @@ int bpf_sysctl_locked(int *, u_int, void
 
 struct bpf_d *bpfilter_lookup(int);
 
+/*
+ * Called holding ``bd_mtx''.
+ */
 void   bpf_attachd(struct bpf_d *, struct bpf_if *);
 void   bpf_detachd(struct bpf_d *);
 void   bpf_resetd(struct bpf_d *);
@@ -260,11 +263,12 @@ bpf_movein(struct uio *uio, u_int linkty
 
 /*
  * Attach file to the bpf interface, i.e. make d listen on bp.
- * Must be called at splnet.
  */
 void
 bpf_attachd(struct bpf_d *d, struct bpf_if *bp)
 {
+   MUTEX_ASSERT_LOCKED(>bd_mtx);
+
/*
 * Point d at bp, and add d to the interface's list of listeners.
 * Finally, point the driver's bpf cookie at the interface so
@@ -287,6 +291,8 @@ bpf_detachd(struct bpf_d *d)
 {
struct bpf_if *bp;
 
+   MUTEX_ASSERT_LOCKED(>bd_mtx);
+
bp = d->bd_bif;
/* Not attached. */
if (bp == NULL)
@@ -313,7 +319,13 @@ bpf_detachd(struct bpf_d *d)
int error;
 
d->bd_promisc = 0;
+
+   bpf_get(d);
+   mtx_leave(>bd_mtx);
error = ifpromisc(bp->bif_ifp, 0);
+   mtx_enter(>bd_mtx);
+   bpf_put(d);
+
if (error && !(error == EINVAL || error == ENODEV))
/*
 * Something is really wrong if we were able to put
@@ -353,6 +365,7 @@ bpfopen(dev_t dev, int flag, int mode, s
bd->bd_unit = unit;
bd->bd_bufsize = bpf_bufsize;
bd->bd_sig = SIGIO;
+   mtx_init(>bd_mtx, IPL_NET);
task_set(>bd_wake_task, bpf_wakeup_cb, bd);
 
if (flag & FNONBLOCK)
@@ -372,15 +385,14 @@ int
 bpfclose(dev_t dev, int flag, int mode, struct proc *p)
 {
struct bpf_d *d;
-   int s;
 
d = bpfilter_lookup(minor(dev));
-   s = splnet();
+   mtx_enter(>bd_mtx);
bpf_detachd(d);
bpf_wakeup(d);
LIST_REMOVE(d, bd_list);
+   mtx_leave(>bd_mtx);
bpf_put(d);
-   splx(s);
 
return (0);
 }
@@ -391,11 +403,13 @@ bpfclose(dev_t dev, int flag, int mode, 
  * Zero the length of the new store buffer.
  */
 #define ROTATE_BUFFERS(d) \
+   KASSERT(d->__in_uiomove == 0); \
+   MUTEX_ASSERT_LOCKED(>bd_mtx); \
(d)->bd_hbuf = (d)->bd_sbuf; \
(d)->bd_hlen = (d)->bd_slen; \
(d)->bd_sbuf = (d)->bd_fbuf; \
(d)->bd_slen = 0; \
-   (d)->bd_fbuf = 0;
+   (d)->bd_fbuf = NULL;
 /*
  *  bpfread - read next chunk of packets from buffers
  */
@@ -403,15 +417,17 @@ int
 bpfread(dev_t dev, struct uio *uio, int ioflag)
 {
struct bpf_d *d;
-   int error;
-   int s;
+   caddr_t hbuf;
+   int hlen, error;
+
+   KERNEL_ASSERT_LOCKED();
 
d = bpfilter_lookup(minor(dev));
if (d->bd_bif == NULL)
return (ENXIO);
 
-   s = splnet();
bpf_get(d);
+   mtx_enter(>bd_mtx);
 
/*
 * Restrict application to use a buffer the same size as
@@ -460,8 +476,8 @@ bpfread(dev_t dev, struct uio

Re: dhcrelay(8): filter BOOTREPLY packets

2016-12-12 Thread Jeremie Courreges-Anglas 
Rafael Zalamena  writes:

> This diff makes dhcrelay(8) drop packets that were not meant for us.
> This is a safety check suggested by jca@ to avoid relaying packets with
> the address of other relays.
>
> ok?

ok

In the commit message please mention that the most likely source of this
kind of packets is the BPF socket.

> Index: dhcrelay.c
> ===
> RCS file: /cvs/src/usr.sbin/dhcrelay/dhcrelay.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 dhcrelay.c
> --- dhcrelay.c8 Dec 2016 19:18:15 -   1.49
> +++ dhcrelay.c8 Dec 2016 19:52:51 -
> @@ -276,6 +276,11 @@ relay(struct interface_info *ip, struct 
>  
>   /* If it's a bootreply, forward it to the client. */
>   if (packet->op == BOOTREPLY) {
> + /* Filter packet that were not meant for us. */
> + if (packet->giaddr.s_addr !=
> + interfaces->primary_address.s_addr)
> + return;
> +
>   bzero(, sizeof(to));
>   if (!(packet->flags & htons(BOOTP_BROADCAST))) {
>   to.sin_addr = packet->yiaddr;
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

umb: aggregate packets on tx

2016-12-12 Thread Gerhard Roth 
The current umb(4) implementation needs one USB transfer for every packet
that is sent. With the following patch, we can now aggregate several
packets from the ifq into one single USB transfer.

This may speed up the tx path. And even if it doesn't, at least it
reduces the number of transfers required.


Gerhard


Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.8
diff -u -p -u -p -r1.8 if_umb.c
--- sys/dev/usb/if_umb.c25 Nov 2016 12:43:26 -  1.8
+++ sys/dev/usb/if_umb.c12 Dec 2016 13:38:34 -
@@ -156,7 +156,7 @@ int  umb_decode_connect_info(struct umb
 int umb_decode_ip_configuration(struct umb_softc *, void *, int);
 voidumb_rx(struct umb_softc *);
 voidumb_rxeof(struct usbd_xfer *, void *, usbd_status);
-int umb_encap(struct umb_softc *, struct mbuf *);
+int umb_encap(struct umb_softc *);
 voidumb_txeof(struct usbd_xfer *, void *, usbd_status);
 voidumb_decap(struct umb_softc *, struct usbd_xfer *);
 
@@ -299,6 +299,7 @@ umb_attach(struct device *parent, struct
 
sc->sc_udev = uaa->device;
sc->sc_ctrl_ifaceno = uaa->ifaceno;
+   ml_init(>sc_tx_ml);
 
/*
 * Some MBIM hardware does not provide the mandatory CDC Union
@@ -583,8 +584,25 @@ umb_ncm_setup(struct umb_softc *sc)
UGETW(np.wLength) == sizeof (np)) {
sc->sc_rx_bufsz = UGETDW(np.dwNtbInMaxSize);
sc->sc_tx_bufsz = UGETDW(np.dwNtbOutMaxSize);
-   } else
+   sc->sc_maxdgram = UGETW(np.wNtbOutMaxDatagrams);
+   sc->sc_align = UGETW(np.wNdpOutAlignment);
+   sc->sc_ndp_div = UGETW(np.wNdpOutDivisor);
+   sc->sc_ndp_remainder = UGETW(np.wNdpOutPayloadRemainder);
+   /* Validate values */
+   if (!powerof2(sc->sc_align) || sc->sc_align == 0 ||
+   sc->sc_align >= sc->sc_tx_bufsz)
+   sc->sc_align = sizeof (uint32_t);
+   if (!powerof2(sc->sc_ndp_div) || sc->sc_ndp_div == 0 ||
+   sc->sc_ndp_div >= sc->sc_tx_bufsz)
+   sc->sc_ndp_div = sizeof (uint32_t);
+   if (sc->sc_ndp_remainder >= sc->sc_ndp_div)
+   sc->sc_ndp_remainder = 0;
+   } else {
sc->sc_rx_bufsz = sc->sc_tx_bufsz = 8 * 1024;
+   sc->sc_maxdgram = 0;
+   sc->sc_align = sc->sc_ndp_div = sizeof (uint32_t);
+   sc->sc_ndp_remainder = 0;
+   }
 }
 
 int
@@ -593,12 +611,12 @@ umb_alloc_xfers(struct umb_softc *sc)
if (!sc->sc_rx_xfer) {
if ((sc->sc_rx_xfer = usbd_alloc_xfer(sc->sc_udev)) != NULL)
sc->sc_rx_buf = usbd_alloc_buffer(sc->sc_rx_xfer,
-   sc->sc_rx_bufsz + MBIM_HDR32_LEN);
+   sc->sc_rx_bufsz);
}
if (!sc->sc_tx_xfer) {
if ((sc->sc_tx_xfer = usbd_alloc_xfer(sc->sc_udev)) != NULL)
sc->sc_tx_buf = usbd_alloc_buffer(sc->sc_tx_xfer,
-   sc->sc_tx_bufsz + MBIM_HDR16_LEN);
+   sc->sc_tx_bufsz);
}
return (sc->sc_rx_buf && sc->sc_tx_buf) ? 1 : 0;
 }
@@ -617,10 +635,7 @@ umb_free_xfers(struct umb_softc *sc)
sc->sc_tx_xfer = NULL;
sc->sc_tx_buf = NULL;
}
-   if (sc->sc_tx_m) {
-   m_freem(sc->sc_tx_m);
-   sc->sc_tx_m = NULL;
-   }
+   ml_purge(>sc_tx_ml);
 }
 
 int
@@ -792,35 +807,91 @@ umb_input(struct ifnet *ifp, struct mbuf
return 1;
 }
 
+static inline int
+umb_align(size_t bufsz, int offs, int alignment, int remainder)
+{
+   size_t   m = alignment - 1;
+   int  align;
+
+   align = (((size_t)offs + m) & ~m) - alignment + remainder;
+   if (align < offs)
+   align += alignment;
+   if (align > bufsz)
+   align = bufsz;
+   return align - offs;
+}
+
+static inline int
+umb_padding(void *buf, size_t bufsz, int offs, int alignment, int remainder)
+{
+   int  nb;
+
+   nb = umb_align(bufsz, offs, alignment, remainder);
+   if (nb > 0)
+   memset(buf + offs, 0, nb);
+   return nb;
+}
+
 void
 umb_start(struct ifnet *ifp)
 {
struct umb_softc *sc = ifp->if_softc;
-   struct mbuf *m_head = NULL;
+   struct mbuf *m = NULL;
+   int  ndgram = 0;
+   int  offs, plen, len, mlen;
+   int  maxalign;
 
if (usbd_is_dying(sc->sc_udev) ||
!(ifp->if_flags & IFF_RUNNING) ||
ifq_is_oactive(>if_snd))
return;
 
-   m_head = ifq_deq_begin(>if_snd);
-   if (m_head == NULL)
-   return;
+   KASSERT(ml_empty(>sc_tx_ml));
 
-   if (!umb_encap(sc, m_head)) {
-

Re: bpf_mtap(9) w/o KERNEL_LOCK

2016-12-12 Thread Martin Pieuchot 
On 07/12/16(Wed) 00:44, Alexander Bluhm wrote:
> On Mon, Nov 28, 2016 at 04:01:14PM +0100, Martin Pieuchot wrote:
> > @@ -717,7 +753,9 @@ bpfioctl(dev_t dev, u_long cmd, caddr_t 
> > *(u_int *)addr = size = bpf_maxbufsize;
> > else if (size < BPF_MINBUFSIZE)
> > *(u_int *)addr = size = BPF_MINBUFSIZE;
> > +   mtx_enter(>bd_mtx);
> > d->bd_bufsize = size;
> > +   mtx_leave(>bd_mtx);
> > }
> > break;
> >  
> 
> bd_bufsize needs protection, but you only add it in BIOCSBLEN,
> you should also put a mutex in BIOCGBLEN.

I'm not sure to understand why.  The value is only written in ioctl(2)
context.  The mutex is here to make sure the value wont change while
bpf_catchpacket() is executed.

> > @@ -778,30 +815,36 @@ bpfioctl(dev_t dev, u_long cmd, caddr_t 
> >  * Get device parameters.
> >  */
> > case BIOCGDLT:
> > +   mtx_enter(>bd_mtx);
> > if (d->bd_bif == NULL)
> > error = EINVAL;
> > else
> > *(u_int *)addr = d->bd_bif->bif_dlt;
> > +   mtx_leave(>bd_mtx);
> > break;
> 
> bd_bif is not protected elsewhere.  bif_dlt is only set during attach.
> So I think you need no mutex here.

Indeed.

> >  
> > /*
> >  * Set device parameters.
> >  */
> > case BIOCSDLT:
> > +   mtx_enter(>bd_mtx);
> > if (d->bd_bif == NULL)
> > error = EINVAL;
> > else
> > error = bpf_setdlt(d, *(u_int *)addr);
> > +   mtx_leave(>bd_mtx);
> > break;
> 
> bd_bif needs not protection, so put the mutex only in the else path.
> Then it is obvious why it is there.

Ack.

> >  
> > /*
> >  * Set interface name.
> >  */
> > case BIOCGETIF:
> > +   mtx_enter(>bd_mtx);
> > if (d->bd_bif == NULL)
> > error = EINVAL;
> > else
> > bpf_ifname(d->bd_bif->bif_ifp, (struct ifreq *)addr);
> > +   mtx_leave(>bd_mtx);
> > break;
> 
> The interface name is not used by bpf without kernel lock.  You do
> not need a mutex here.

Ack.

> > @@ -1151,6 +1197,8 @@ filt_bpfread(struct knote *kn, long hint
> >  {
> > struct bpf_d *d = kn->kn_hook;
> >  
> > +   KERNEL_ASSERT_LOCKED();
> > +
> > kn->kn_data = d->bd_hlen;
> > if (d->bd_immediate)
> > kn->kn_data += d->bd_slen;
> 
> You need the mutex here to access bd_slen.

Ack.

> > @@ -1232,12 +1279,10 @@ _bpf_mtap(caddr_t arg, const struct mbuf
> > if (!gottime++)
> > microtime();
> >  
> > -   KERNEL_LOCK();
> > -   s = splnet();
> > +   mtx_enter(>bd_mtx);
> > bpf_catchpacket(d, (u_char *)m, pktlen, slen, cpfn,
> > );
> > -   splx(s);
> > -   KERNEL_UNLOCK();
> > +   mtx_leave(>bd_mtx);
> >  
> > if (d->bd_fildrop)
> > drop = 1;
> 
> bd_fildrop can be changed by ioctl.  Can we expect to read the
> correct value without a memory barrier?
> 
> Same question applies to bd_dirfilt.

We probably need some.  I guess that the actual code works because these
events happen occasionally.

> bd_rcount is a long value incremented atomically in _bpf_mtap().
> It is read from an other CPU with kernel lock during an ioctl.  Is
> this sufficient to get a consisten value?  Do we also need an atomic
> operation when reading it?

Do we need a consistent value?

> > --- net/bpfdesc.h   22 Aug 2016 10:40:36 -  1.31
> > +++ net/bpfdesc.h   28 Nov 2016 14:51:06 -
> > @@ -56,15 +56,17 @@ struct bpf_d {
> >  *   fbuf (free) - When read is done, put cluster here.
> >  * On receiving, if sbuf is full and fbuf is 0, packet is dropped.
> >  */
> > +   struct mutexbd_mtx; /* protect buffer slots below */
> > caddr_t bd_sbuf;/* store slot */
> > caddr_t bd_hbuf;/* hold slot */
> > caddr_t bd_fbuf;/* free slot */
> > int bd_slen;/* current length of store buffer */
> > int bd_hlen;/* current length of hold buffer */
> > -
> > int bd_bufsize; /* absolute length of buffers */
> 
> So bd_mtx is protecting the fields above.  Everything below must
> either be not accessed without kernel lock, or only be set during
> attach.  Is that correct or do I have a wrong understanding of the
> SMP model?

I'm not sure I understood your sentence, but I think you got the model
right :)

> > -   struct bpf_if * bd_bif; /* interface descriptor */
> > +   int __in_uiomove;
> 
> Why do you prefix it with __ ?  bd_uiomove would be more consistent.

Because it's a

Re: efifb(4): WSDISPLAYIO_{GET,SET}PARAM ioctl support

2016-12-12 Thread YASUOKA Masahiko 
comitted.  Thanks!

On Sun, 11 Dec 2016 10:32:59 +0100
Anton Lindqvist  wrote:
> This allows the brightness on my Dell Latitude 3160 to be adjusted
> through wsconsctl(1).
> 
> Index: arch/amd64/amd64/efifb.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/efifb.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 efifb.c
> --- arch/amd64/amd64/efifb.c  21 Jun 2016 15:24:55 -  1.9
> +++ arch/amd64/amd64/efifb.c  11 Dec 2016 09:25:04 -
> @@ -104,6 +104,9 @@ intefifb_load_font(void *, void *, str
>  
>  struct cb_framebuffer *cb_find_fb(paddr_t);
>  
> +extern int   (*ws_get_param)(struct wsdisplay_param *);
> +extern int   (*ws_set_param)(struct wsdisplay_param *);
> +
>  const struct cfattach efifb_ca = {
>   sizeof(struct efifb_softc), efifb_match, efifb_attach, NULL
>  };
> @@ -235,6 +238,16 @@ efifb_ioctl(void *v, u_long cmd, caddr_t
>   struct wsdisplay_fbinfo *wdf;
>  
>   switch (cmd) {
> + case WSDISPLAYIO_GETPARAM:
> + if (ws_get_param != NULL)
> + return (*ws_get_param)((struct wsdisplay_param *)data);
> + else
> + return (-1);
> + case WSDISPLAYIO_SETPARAM:
> + if (ws_set_param != NULL)
> + return (*ws_set_param)((struct wsdisplay_param *)data);
> + else
> + return (-1);
>   case WSDISPLAYIO_GTYPE:
>   *(u_int *)data = WSDISPLAY_TYPE_EFIFB;
>   break;
>

ifconfig and tunnels using ipv6 link local addresses

2016-12-12 Thread David Gwynne 
the system is inconsistent wrt to tunnels and ipv6 link local
addresses.

there are three things to consider here:

1. configuring addresses on a tunnel
2. using addresses to encapsulate traffic
3. reading the addresses a tunnel uses

the ioctls used for 1 and 3 use sockaddr_in6 to pass v6 addresses
in and out of the kernel for tunnel configuration. sin6 structures
have a sin6_scope_id member that is supposed to represent the scope.

i get the impression sin6_scope_id wasnt always there thoughway.
kame had (has) a hack where it embeds the scope id into bits that
are guaranteed to be zero.

both ifconfig and the kernel have code to pack sin6_scope_id into
a v6 address, and to unpack it later.

currently 1 uses getaddrinfo to parse an address on the command
line and submit it to the kernel. getaddrinfo reads scope (eg, %ix0)
and puts it into sin6_scope_id.

for some reason 3 currently assumes that needs to unpack a kame
packed address and. that means it reads from the "zero" bits in the
address into sin6_scope_id.

1 and 3 are therefore inconsistent. 1 submits a nice neat sin6_scope_id
to the kernel, but 3 assumes its a kame packed address.

to resolve this im suggesting we pick 1 as the right way to do
things. sin6_scope_id is portable, and less confusing. i havent met
anyone who likes the kame hack.

for 2, ipv6 packets end up in ip6_output, and ip6_output relies on
scope embedded into link local addresses. at the moment there are
3 tunnel interfaces that let you use v6 addresses for the enpoints:
gif, etherip, and vxlan. they all use the sockaddr_sin6 from userland
to fill in a v6 header, but none of them use the scope id.

the diff below should fix all this. the most important fix is vxlan.

ok?

Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.333
diff -u -p -r1.333 ifconfig.c
--- sbin/ifconfig/ifconfig.c10 Nov 2016 14:36:03 -  1.333
+++ sbin/ifconfig/ifconfig.c12 Dec 2016 09:44:48 -
@@ -2873,8 +2873,6 @@ phys_status(int force)
(void) strlcpy(req.iflr_name, name, sizeof(req.iflr_name));
if (ioctl(s, SIOCGLIFPHYADDR, (caddr_t)) < 0)
return;
-   if (req.addr.ss_family == AF_INET6)
-   in6_fillscopeid((struct sockaddr_in6 *));
if (getnameinfo((struct sockaddr *), req.addr.ss_len,
psrcaddr, sizeof(psrcaddr), 0, 0, niflag) != 0)
strlcpy(psrcaddr, "", sizeof(psrcaddr));
@@ -2883,10 +2881,8 @@ phys_status(int force)
 
if (req.dstaddr.ss_family == AF_INET)
dstport = ((struct sockaddr_in *))->sin_port;
-   else if (req.dstaddr.ss_family == AF_INET6) {
-   in6_fillscopeid((struct sockaddr_in6 *));
+   else if (req.dstaddr.ss_family == AF_INET6)
dstport = ((struct sockaddr_in6 *))->sin6_port;
-   }
if (getnameinfo((struct sockaddr *), req.dstaddr.ss_len,
pdstaddr, sizeof(pdstaddr), 0, 0, niflag) != 0)
strlcpy(pdstaddr, "", sizeof(pdstaddr));
Index: sys/net/if_etherip.c
===
RCS file: /cvs/src/sys/net/if_etherip.c,v
retrieving revision 1.7
diff -u -p -r1.7 if_etherip.c
--- sys/net/if_etherip.c13 Apr 2016 11:41:15 -  1.7
+++ sys/net/if_etherip.c12 Dec 2016 09:44:48 -
@@ -514,18 +514,19 @@ ip6_etherip_output(struct ifnet *ifp, st
struct sockaddr_in6 *src, *dst;
struct etherip_header *eip;
struct ip6_hdr *ip6;
+   int error;
 
src = (struct sockaddr_in6 *)>sc_src;
dst = (struct sockaddr_in6 *)>sc_dst;
 
if (src == NULL || dst == NULL ||
src->sin6_family != AF_INET6 || dst->sin6_family != AF_INET6) {
-   m_freem(m);
-   return EAFNOSUPPORT;
+   error = EAFNOSUPPORT;
+   goto drop;
}
if (IN6_IS_ADDR_UNSPECIFIED(>sin6_addr)) {
-   m_freem(m);
-   return ENETUNREACH;
+   error = ENETUNREACH;
+   goto drop;
}
 
m->m_flags &= ~(M_BCAST|M_MCAST);
@@ -552,8 +553,12 @@ ip6_etherip_output(struct ifnet *ifp, st
ip6->ip6_nxt  = IPPROTO_ETHERIP;
ip6->ip6_hlim = ip6_defhlim;
ip6->ip6_plen = htons(m->m_pkthdr.len - sizeof(struct ip6_hdr));
-   ip6->ip6_src  = src->sin6_addr;
-   ip6->ip6_dst = dst->sin6_addr;
+   error = in6_embedscope(>ip6_src, src, NULL);
+   if (error != 0)
+   goto drop;
+   error = in6_embedscope(>ip6_dst, dst, NULL);
+   if (error != 0)
+   goto drop;
 
m->m_pkthdr.ph_rtableid = sc->sc_rdomain;
 
@@ -565,6 +570,10 @@ ip6_etherip_output(struct ifnet *ifp, st
(sizeof(struct ip6_hdr) + sizeof(struct etherip_header)));
 
return ip6_output(m, 0, NULL, IPV6_MINMTU, 0, NULL);
+
+drop:
+   m_freem(m);
+   return

Re: route(8) flush fix

2016-12-12 Thread Stefan Sperling 
On Mon, Dec 12, 2016 at 10:53:42AM +0100, Martin Pieuchot wrote:
> This is a just a cosmetic fix.  Currently the netmask of flushed routes
> is not printed correctly.  The diff below fixes that.
> 
> ok?

ok stsp@

> 
> Index: route.c
> ===
> RCS file: /cvs/src/sbin/route/route.c,v
> retrieving revision 1.192
> diff -u -p -r1.192 route.c
> --- route.c   24 Sep 2016 19:36:49 -  1.192
> +++ route.c   9 Dec 2016 13:20:04 -
> @@ -385,13 +385,17 @@ flushroutes(int argc, char **argv)
>   if (verbose)
>   print_rtmsg(rtm, rlen);
>   else {
> + struct sockaddr *mask, *rti_info[RTAX_MAX];
> +
>   sa = (struct sockaddr *)(next + rtm->rtm_hdrlen);
> - printf("%-20.20s ", rtm->rtm_flags & RTF_HOST ?
> - routename(sa) : netname(sa, NULL)); /* XXX extract
> -netmask */
> - sa = (struct sockaddr *)
> - (ROUNDUP(sa->sa_len) + (char *)sa);
> - printf("%-20.20s ", routename(sa));
> +
> + get_rtaddrs(rtm->rtm_addrs, sa, rti_info);
> +
> + sa = rti_info[RTAX_DST];
> + mask = rti_info[RTAX_NETMASK];
> +
> + p_sockaddr(sa, mask, rtm->rtm_flags, 20);
> + p_sockaddr(rti_info[RTAX_GATEWAY], NULL, RTF_HOST, 20);
>   printf("done\n");
>   }
>   }
> Index: show.c
> ===
> RCS file: /cvs/src/sbin/route/show.c,v
> retrieving revision 1.107
> diff -u -p -r1.107 show.c
> --- show.c5 Sep 2016 14:23:38 -   1.107
> +++ show.c9 Dec 2016 13:19:44 -
> @@ -100,7 +100,6 @@ intWID_DST(int);
>  void  pr_rthdr(int);
>  void  p_rtentry(struct rt_msghdr *);
>  void  pr_family(int);
> -void  p_sockaddr(struct sockaddr *, struct sockaddr *, int, int);
>  void  p_sockaddr_mpls(struct sockaddr *, struct sockaddr *, int, int);
>  void  p_flags(int, char *);
>  char *routename4(in_addr_t);
> @@ -224,7 +223,7 @@ pr_rthdr(int af)
>   }
>  }
>  
> -static void
> +void
>  get_rtaddrs(int addrs, struct sockaddr *sa, struct sockaddr **rti_info)
>  {
>   int i;
> Index: show.h
> ===
> RCS file: /cvs/src/sbin/route/show.h,v
> retrieving revision 1.11
> diff -u -p -r1.11 show.h
> --- show.h18 Jul 2015 00:05:02 -  1.11
> +++ show.h9 Dec 2016 13:19:55 -
> @@ -28,7 +28,9 @@ union sockunion {
>   struct sockaddr_mplssmpls;
>  };
>  
> +void  get_rtaddrs(int, struct sockaddr *, struct sockaddr **);
>  void  p_rttables(int, u_int, int, char);
> +void  p_sockaddr(struct sockaddr *, struct sockaddr *, int, int);
>  char *routename(struct sockaddr *);
>  char *netname(struct sockaddr *, struct sockaddr *);
>  char *mpls_op(u_int32_t);
>

Re: Proxy ARP and npppd (tun)

2016-12-12 Thread Martin Pieuchot 
On 09/12/16(Fri) 19:42, Erik Lax wrote:
> Hi,
> 
> In previous OpenBSD versions (5.9 and eariler) it was possible to do
> proxy-arp with the npppd server if the proxy-arp was setup before the
> npppd connection was made. As of 6.0 (and todays snapshots) proxy arp
> and npppd (tun interfaces) seems to be broken.
> 
> The behavior is now as such;
> 
> - if an proxy arp entry exists (on a given interface), then incoming
> packets will be sent out again on the same interface (instead of
> forwarded to the tun interface)
> 
> Setup.
> 
> 1. Adding the proxy-arp entry 10.2.50.123 (the npppd/tun0 client ip)
> 
> arp -s 10.2.50.123 00:50:51:b2:e4:c1 permanent pub
> 
> 2. Connect the client, send ping to host on em0, incoming icmp replys
> are sent out on em0 again instead of tun0

Could you include the output of "netstat -rnf inet" at this point?

> A quirk?
> 
> If the arp entry is deleted (arp -d 10.2.50.124) on the openbsd host, it
> starts to work temporarily (the packet is forwarded to the tun
> interface) but only because the remote host has an arp cache.

Could you include the same output at this point?

> Is there an other way of doing this?

There's a regression somewhere, thanks for reporting it.  a dmesg would
also help.

Re: improve gre handling in tcpdump

2016-12-12 Thread Stefan Sperling 
On Mon, Dec 12, 2016 at 01:02:55PM +1000, David Gwynne wrote:
> gre can do more things than tcpdump currently thinks it can.
> 
> specifically, gre can be carried by ipv6, and it can encapsulate
> more than just ip and ppp packets.
> 
> as such, this tells tcpdump to look at gre inside ipv6 packets.
> 
> gre uses ethertypes to represent what protocol it contains, so
> instead of rolling a gre specific version of ip and ppp protocol
> types, just reuse the ether ones.
> 
> also tell tcpdump that gre can contains ipv6, ethernet, and mpls.
> 
> NVGRE is basically a constrained gre header (ie, must be version
> 0, must only have the K bit set, must be transether), so this detects
> that and prints the NVGRE interpretation of the Key field. that
> makes the VSID in NVGRE packets easier to see.
> 
> theres some tweaks to output so it looks ok with and without the
> -v optarg.
> 
> ok?

ok stsp@

> 
> Index: print-gre.c
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/print-gre.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 print-gre.c
> --- print-gre.c   5 Nov 2015 11:55:21 -   1.11
> +++ print-gre.c   12 Dec 2016 02:48:21 -
> @@ -39,6 +39,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  #include 
>  #include 
>  
> @@ -55,13 +57,15 @@
>  #define  GRE_AP  0x0080  /* acknowledgment# present */
>  #define  GRE_VERS0x0007  /* protocol version */
>  
> -#define  GREPROTO_IP 0x0800  /* IP */
> -#define  GREPROTO_PPP0x880b  /* PPTP */
> -
>  /* source route entry types */
>  #define  GRESRE_IP   0x0800  /* IP */
>  #define  GRESRE_ASN  0xfffe  /* ASN */
>  
> +#define NVGRE_VSID_MASK  0xff00U
> +#define NVGRE_VSID_SHIFT 8
> +#define NVGRE_FLOWID_MASK0x00ffU
> +#define NVGRE_FLOWID_SHIFT   0
> +
>  void gre_print_0(const u_char *, u_int);
>  void gre_print_1(const u_char *, u_int);
>  void gre_sre_print(u_int16_t, u_int8_t, u_int8_t, const u_char *, u_int);
> @@ -82,14 +86,17 @@ gre_print(const u_char *bp, u_int length
>   }
>   vers = EXTRACT_16BITS(bp) & GRE_VERS;
>  
> - if (vers == 0)
> + switch (vers) {
> + case 0:
>   gre_print_0(bp, len);
> - else if (vers == 1)
> + break;
> + case 1:
>   gre_print_1(bp, len);
> - else
> + break;
> + default:
>   printf("gre-unknown-version=%u", vers);
> - return;
> -
> + break;
> + }
>  }
>  
>  void
> @@ -114,6 +121,8 @@ gre_print_0(const u_char *bp, u_int leng
>   if (len < 2)
>   goto trunc;
>   prot = EXTRACT_16BITS(bp);
> + printf("%s", etherproto_string(prot));
> +
>   len -= 2;
>   bp += 2;
>  
> @@ -121,21 +130,32 @@ gre_print_0(const u_char *bp, u_int leng
>   if (len < 2)
>   goto trunc;
>   if (vflag)
> - printf("sum 0x%x ", EXTRACT_16BITS(bp));
> + printf(" sum 0x%x", EXTRACT_16BITS(bp));
>   bp += 2;
>   len -= 2;
>  
>   if (len < 2)
>   goto trunc;
> - printf("off 0x%x ", EXTRACT_16BITS(bp));
> + printf(" off 0x%x", EXTRACT_16BITS(bp));
>   bp += 2;
>   len -= 2;
>   }
>  
>   if (flags & GRE_KP) {
> + uint32_t key, vsid;
> +
>   if (len < 4)
>   goto trunc;
> - printf("key=0x%x ", EXTRACT_32BITS(bp));
> + key = EXTRACT_32BITS(bp);
> +
> + /* maybe NVGRE? */
> + if (flags == (GRE_KP | 0) && prot == ETHERTYPE_TRANSETHER) {
> + vsid = (key & NVGRE_VSID_MASK) >> NVGRE_VSID_SHIFT;
> + printf(" NVGRE vsid=%u (0x%x)+flowid=0x%02x /",
> + vsid, vsid,
> + (key & NVGRE_FLOWID_MASK) >> NVGRE_FLOWID_SHIFT);
> + }
> + printf(" key=%u (0x%x)", key, key);
>   bp += 4;
>   len -= 4;
>   }
> @@ -143,7 +163,7 @@ gre_print_0(const u_char *bp, u_int leng
>   if (flags & GRE_SP) {
>   if (len < 4)
>   goto trunc;
> - printf("seq %u ", EXTRACT_32BITS(bp));
> + printf(" seq %u", EXTRACT_32BITS(bp));
>   bp += 4;
>   len -= 4;
>   }
> @@ -174,10 +194,21 @@ gre_print_0(const u_char *bp, u_int leng
>   }
>   }
>  
> + printf(": ");
> +
>   switch (prot) {
> - case GREPROTO_IP:
> + case ETHERTYPE_IP:
>   ip_print(bp, len);
>   break;
> + case ETHERTYPE_IPV6:
> + ip6_print(bp, len);
> + break;
> + case ETHERTYPE_MPLS:
> + mpls_print(bp, len);
> + break;
> + case ETHERTYPE_TRANSETHER:
> +

route(8) flush fix

2016-12-12 Thread Martin Pieuchot 
This is a just a cosmetic fix.  Currently the netmask of flushed routes
is not printed correctly.  The diff below fixes that.

ok?

Index: route.c
===
RCS file: /cvs/src/sbin/route/route.c,v
retrieving revision 1.192
diff -u -p -r1.192 route.c
--- route.c 24 Sep 2016 19:36:49 -  1.192
+++ route.c 9 Dec 2016 13:20:04 -
@@ -385,13 +385,17 @@ flushroutes(int argc, char **argv)
if (verbose)
print_rtmsg(rtm, rlen);
else {
+   struct sockaddr *mask, *rti_info[RTAX_MAX];
+
sa = (struct sockaddr *)(next + rtm->rtm_hdrlen);
-   printf("%-20.20s ", rtm->rtm_flags & RTF_HOST ?
-   routename(sa) : netname(sa, NULL)); /* XXX extract
-  netmask */
-   sa = (struct sockaddr *)
-   (ROUNDUP(sa->sa_len) + (char *)sa);
-   printf("%-20.20s ", routename(sa));
+
+   get_rtaddrs(rtm->rtm_addrs, sa, rti_info);
+
+   sa = rti_info[RTAX_DST];
+   mask = rti_info[RTAX_NETMASK];
+
+   p_sockaddr(sa, mask, rtm->rtm_flags, 20);
+   p_sockaddr(rti_info[RTAX_GATEWAY], NULL, RTF_HOST, 20);
printf("done\n");
}
}
Index: show.c
===
RCS file: /cvs/src/sbin/route/show.c,v
retrieving revision 1.107
diff -u -p -r1.107 show.c
--- show.c  5 Sep 2016 14:23:38 -   1.107
+++ show.c  9 Dec 2016 13:19:44 -
@@ -100,7 +100,6 @@ int  WID_DST(int);
 voidpr_rthdr(int);
 voidp_rtentry(struct rt_msghdr *);
 voidpr_family(int);
-voidp_sockaddr(struct sockaddr *, struct sockaddr *, int, int);
 voidp_sockaddr_mpls(struct sockaddr *, struct sockaddr *, int, int);
 voidp_flags(int, char *);
 char   *routename4(in_addr_t);
@@ -224,7 +223,7 @@ pr_rthdr(int af)
}
 }
 
-static void
+void
 get_rtaddrs(int addrs, struct sockaddr *sa, struct sockaddr **rti_info)
 {
int i;
Index: show.h
===
RCS file: /cvs/src/sbin/route/show.h,v
retrieving revision 1.11
diff -u -p -r1.11 show.h
--- show.h  18 Jul 2015 00:05:02 -  1.11
+++ show.h  9 Dec 2016 13:19:55 -
@@ -28,7 +28,9 @@ union sockunion {
struct sockaddr_mplssmpls;
 };
 
+voidget_rtaddrs(int, struct sockaddr *, struct sockaddr **);
 voidp_rttables(int, u_int, int, char);
+voidp_sockaddr(struct sockaddr *, struct sockaddr *, int, int);
 char   *routename(struct sockaddr *);
 char   *netname(struct sockaddr *, struct sockaddr *);
 char   *mpls_op(u_int32_t);