Le Tue, 7 Jun 2016 10:48:22 +0200,
Martin Pieuchot <m...@openbsd.org> a écrit :

> On 06/06/16(Mon) 23:52, Vincent Gross wrote:
> > On Mon, 6 Jun 2016 17:33:36 +0100
> > Stuart Henderson <s...@spacehopper.org> wrote:
> >   
> > > On 2016/06/06 16:15, Vincent Gross wrote:  
> > > > When sending ARP requests, or when writing to a bpf handle (as
> > > > when sending DHCP Discover), we bypass pf(4) so we have no way
> > > > to define the priority (m->m_pkthdr.pf.prio) of the outgoing
> > > > packets.  
> > [...]  
> > > > 
> > > > This diff adds
> > > > 1) an if_llprio field to struct ifnet    
> > > 
> > > struct if_data.. this is used by enough ports that changing the
> > > abi  
> > [...]  
> > >   
> > > > diff --git a/sbin/ifconfig/ifconfig.8
> > > > b/sbin/ifconfig/ifconfig.8    
> > > 
> > > BTW. patch warns about offsets if you apply this to -current.
> > >   
> > [...]  
> > > 
> > > Other than these points, it seems a useful thing to do, pppoe
> > > could use it too.
> > > 
> > > I wonder what these broken ISP devices are that require the
> > > priority field in the vlan frame header to be 0 (aka "prio 1")...
> > >   
> > 
> > r2 below. I moved if_llprio from if_data to struct ifnet, and went
> > from u_char to u_int8_t. I also added a bound check in ifioctl().
> > 
> > Comments ? ok ?  
> 
> Could you explain me why our default prio is 3?
> 

It's how henning@ set things up when integrating the new queuing mechanism.
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/kern/uipc_mbuf.c#rev1.160

> Is there any use for this apart for vlan(4) interfaces?

AFAICT, no. 

> Should it
> really be part of "struct ifnet" ?
> 

sthen@ pointed out that struct if_data was heavily used by our ports, and
that such a change would require a version bump. Now, I may have overlooked
a better place for it.

I don't think we should make a special case for vlan(4), this kind of detail
do not belong to the arp(4) or bpf(4) layer.

> I also find weird to see a field inside ``m_pkthdr.pf'' being used
> without pf(4).
> 
> > Index: sbin/ifconfig/ifconfig.8
> > ===================================================================
> > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> > retrieving revision 1.267
> > diff -u -p -r1.267 ifconfig.8
> > --- sbin/ifconfig/ifconfig.8        6 Apr 2016 10:07:14
> > -0000       1.267 +++ sbin/ifconfig/ifconfig.8      6 Jun 2016
> > 21:43:46 -0000 @@ -327,6 +327,10 @@ Disable special processing at
> > the link l Change the link layer address (MAC address) of the
> > interface. This should be specified as six colon-separated hex
> > values, or can be chosen randomly.
> > +.It Cm llprio Ar prio
> > +Set the priority for link layer communications
> > +.Pf ( Xr arp 4 ,
> > +.Xr bpf 4 ) .
> >  .It Cm media Op Ar type
> >  Set the media type of the interface to
> >  .Ar type .
> > Index: sbin/ifconfig/ifconfig.c
> > ===================================================================
> > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> > retrieving revision 1.322
> > diff -u -p -r1.322 ifconfig.c
> > --- sbin/ifconfig/ifconfig.c        3 May 2016 17:52:33
> > -0000       1.322 +++ sbin/ifconfig/ifconfig.c      6 Jun 2016
> > 21:43:46 -0000 @@ -135,6 +135,7 @@ char     name[IFNAMSIZ];
> >  int        flags, xflags, setaddr, setipdst, doalias;
> >  u_long     metric, mtu;
> >  int        rdomainid;
> > +int        llprio;
> >  int        clearaddr, s;
> >  int        newaddr = 0;
> >  int        af = AF_INET;
> > @@ -157,6 +158,7 @@ void    addaf(const char *, int);
> >  void       removeaf(const char *, int);
> >  void       setifbroadaddr(const char *, int);
> >  void       setifmtu(const char *, int);
> > +void       setifllprio(const char *, int);
> >  void       setifnwid(const char *, int);
> >  void       setifbssid(const char *, int);
> >  void       setifnwkey(const char *, int);
> > @@ -521,6 +523,7 @@ const struct    cmd {
> >     { "instance",   NEXTARG,        A_MEDIAINST,
> > setmediainst }, { "inst",   NEXTARG,
> > A_MEDIAINST,        setmediainst }, { "lladdr",
> > NEXTARG,    0,              setiflladdr },
> > +   { "llprio",     NEXTARG,        0,
> > setifllprio }, { NULL, /*src*/      0,
> > 0,          setifaddr }, { NULL, /*dst*/
> > 0,          0,              setifdstaddr },
> > { NULL, /*illegal*/0,               0,              NULL }, @@
> > -854,6 +857,11 @@ getinfo(struct ifreq *ifr, int create) else
> >             rdomainid = ifr->ifr_rdomainid;
> >  #endif
> > +   if (ioctl(s, SIOCGIFLLPRIO, (caddr_t)ifr) < 0)
> > +           llprio = 0;
> > +   else
> > +           llprio = ifr->ifr_llprio;
> > +
> >     return (0);
> >  }
> >  
> > @@ -1411,6 +1419,21 @@ setifmtu(const char *val, int d)
> >  
> >  /* ARGSUSED */
> >  void
> > +setifllprio(const char *val, int d)
> > +{
> > +   const char *errmsg = NULL;
> > +
> > +   (void) strlcpy(ifr.ifr_name, name, sizeof(ifr.ifr_name));
> > +
> > +   ifr.ifr_mtu = strtonum(val, 0, UCHAR_MAX, &errmsg);
> > +   if (errmsg)
> > +           errx(1, "mtu %s: %s", val, errmsg);
> > +   if (ioctl(s, SIOCSIFLLPRIO, (caddr_t)&ifr) < 0)
> > +           warn("SIOCSIFLLPRIO");
> > +}
> > +
> > +/* ARGSUSED */
> > +void
> >  setifgroup(const char *group_name, int dummy)
> >  {
> >     struct ifgroupreq ifgr;
> > @@ -2894,6 +2917,7 @@ status(int link, struct sockaddr_dl *sdl
> >             printf(" metric %lu", metric);
> >     if (mtu)
> >             printf(" mtu %lu", mtu);
> > +   printf(" llprio %lu", llprio);
> >     putchar('\n');
> >  #ifndef SMALL
> >     if (showcapsflag)
> > Index: sys/net/bpf.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/bpf.c,v
> > retrieving revision 1.141
> > diff -u -p -r1.141 bpf.c
> > --- sys/net/bpf.c   18 May 2016 03:46:03 -0000      1.141
> > +++ sys/net/bpf.c   6 Jun 2016 21:43:48 -0000
> > @@ -561,6 +561,7 @@ bpfwrite(dev_t dev, struct uio *uio, int
> >     }
> >  
> >     m->m_pkthdr.ph_rtableid = ifp->if_rdomain;
> > +   m->m_pkthdr.pf.prio = ifp->if_llprio;
> >  
> >     if (d->bd_hdrcmplt && dst.ss_family == AF_UNSPEC)
> >             dst.ss_family = pseudo_AF_HDRCMPLT;
> > Index: sys/net/if.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.433
> > diff -u -p -r1.433 if.c
> > --- sys/net/if.c    18 May 2016 03:46:03 -0000      1.433
> > +++ sys/net/if.c    6 Jun 2016 21:43:48 -0000
> > @@ -536,6 +536,7 @@ if_attach_common(struct ifnet *ifp)
> >         M_TEMP, M_WAITOK|M_ZERO);
> >     ifp->if_linkstatetask =
> > malloc(sizeof(*ifp->if_linkstatetask), M_TEMP, M_WAITOK|M_ZERO);
> > +   ifp->if_llprio = IFQ_DEFPRIO;
> >  
> >     SRPL_INIT(&ifp->if_inputs);
> >  }
> > @@ -1986,6 +1987,18 @@ ifioctl(struct socket *so, u_long cmd, c
> >             }
> >  
> >             ifnewlladdr(ifp);
> > +           break;
> > +
> > +   case SIOCGIFLLPRIO:
> > +           ifr->ifr_llprio = ifp->if_llprio;
> > +           break;
> > +
> > +   case SIOCSIFLLPRIO:
> > +           if ((error = suser(p, 0)))
> > +                   return (error);
> > +           if (ifr->ifr_llprio > UCHAR_MAX)
> > +                   return (EINVAL);
> > +           ifp->if_llprio = ifr->ifr_llprio;
> >             break;
> >  
> >     default:
> > Index: sys/net/if.h
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if.h,v
> > retrieving revision 1.176
> > diff -u -p -r1.176 if.h
> > --- sys/net/if.h    2 Mar 2016 00:00:16 -0000       1.176
> > +++ sys/net/if.h    6 Jun 2016 21:43:48 -0000
> > @@ -378,6 +378,7 @@ struct  ifreq {
> >  #define ifr_ttl            ifr_ifru.ifru_metric    /*
> > tunnel TTL (overload) */ #define    ifr_data
> > ifr_ifru.ifru_data  /* for use by interface */ #define
> > ifr_index   ifr_ifru.ifru_index     /* interface index */
> > +#define ifr_llprio ifr_ifru.ifru_metric    /* link
> > layer priority */ }; 
> >  struct ifaliasreq {
> > Index: sys/net/if_var.h
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if_var.h,v
> > retrieving revision 1.71
> > diff -u -p -r1.71 if_var.h
> > --- sys/net/if_var.h        15 Apr 2016 05:05:21 -0000      1.71
> > +++ sys/net/if_var.h        6 Jun 2016 21:43:48 -0000
> > @@ -131,6 +131,7 @@ struct ifnet
> > {                           /* and the entries */ char
> > if_description[IFDESCRSIZE]; /* interface description */
> > u_short     if_rtlabelid;           /* next route label */
> > u_int8_t if_priority;
> > +   u_int8_t if_llprio;             /* link layer priority
> > */ struct   timeout *if_slowtimo;   /* watchdog timeout */
> >     struct  task *if_watchdogtask;  /* watchdog
> > task */ struct      task *if_linkstatetask; /* task to do route
> > updates */ Index: sys/netinet/if_ether.c
> > ===================================================================
> > RCS file: /cvs/src/sys/netinet/if_ether.c,v
> > retrieving revision 1.213
> > diff -u -p -r1.213 if_ether.c
> > --- sys/netinet/if_ether.c  6 Jun 2016 07:07:11 -0000
> > 1.213 +++ sys/netinet/if_ether.c    6 Jun 2016 21:43:48 -0000
> > @@ -235,6 +235,7 @@ arprequest(struct ifnet *ifp, u_int32_t 
> >     m->m_len = sizeof(*ea);
> >     m->m_pkthdr.len = sizeof(*ea);
> >     m->m_pkthdr.ph_rtableid = ifp->if_rdomain;
> > +   m->m_pkthdr.pf.prio = ifp->if_llprio;
> >     MH_ALIGN(m, sizeof(*ea));
> >     ea = mtod(m, struct ether_arp *);
> >     eh = (struct ether_header *)sa.sa_data;
> > @@ -832,6 +833,7 @@ revarprequest(struct ifnet *ifp)
> >             return;
> >     m->m_len = sizeof(*ea);
> >     m->m_pkthdr.len = sizeof(*ea);
> > +   m->m_pkthdr.pf.prio = ifp->if_llprio;
> >     MH_ALIGN(m, sizeof(*ea));
> >     ea = mtod(m, struct ether_arp *);
> >     eh = (struct ether_header *)sa.sa_data;
> > Index: sys/sys/sockio.h
> > ===================================================================
> > RCS file: /cvs/src/sys/sys/sockio.h,v
> > retrieving revision 1.64
> > diff -u -p -r1.64 sockio.h
> > --- sys/sys/sockio.h        31 May 2016 22:35:02 -0000      1.64
> > +++ sys/sys/sockio.h        6 Jun 2016 21:43:48 -0000
> > @@ -202,6 +202,9 @@
> >  #define SIOCGIFPARENT      _IOWR('i', 179, struct if_parent) /*
> > get parent if */ #define SIOCDIFPARENT      _IOW('i', 180, struct
> > ifreq)      /* del parent if */ 
> > +#define    SIOCSIFLLPRIO   _IOW('i', 181, struct
> > ifreq)      /* set ifnet llprio */ +#define
> > SIOCGIFLLPRIO       _IOWR('i', 182, struct ifreq)   /* get
> > ifnet llprio */ + #define   SIOCSVH         _IOWR('i',
> > 245, struct ifreq)  /* set carp param */ #define
> > SIOCGVH             _IOWR('i', 246, struct ifreq)   /* get
> > carp param */ 

Reply via email to