[PATCH] Huawei E8372 USB Mobile Broadband in HiLink Mode, Generic HiLink Mode Logic
Hello tech@,
Given this is my first email to the list, and the first patch I have
submitted to the OpenBSD project, I am very open to feedback on
the attached patch and context provided!
Below I have included a patch I have been working on to modify some of
the logic when handling E-series Huawei USB mobile broadband dongles.
These devices are able to operate in several modes, the most compatible
and performant mode being as a USB networking device ('HiLink' mode),
providing a NAT'ed connection to the mobile broadband service of the
connected provider.
Using this mode requires no additional software, other than a DHCP
client in OpenBSD. Even that is optional, give you could statically
configure the interface.
This patch refactors lightly some of the logic around handling E-Series
devices, and moves the original device definitions for the E303 to a
generic 'E-Series' device definition, given all E-Series devices seem to
share the parts of the intialisation and the USB product and vendor IDs
which OpenBSD currently assigns to the E303 device.
I have also added additional logic to identify the E8372, which is the
device I am testing with, as additional logic is required to switch it
into HiLink mode. This should work for other E-Series HiLink devices
supporting the same mode. If anyone with one (or more!) of these
devices (say, the E3372, which appears to be almost identical)
could test this patch, it would be very much appreciated.
I have tested this patch and it applies cleanly against -current, 6.4
and 6.3. I am currently using this on the 6.3 kernel as my
home router, and so far it has proved reliable.
Best Regards,
James "ec0" Hebden
Patch follows -
Index: dev/usb/if_cdce.c
===
RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v
retrieving revision 1.75
diff -u -p -u -p -r1.75 if_cdce.c
--- dev/usb/if_cdce.c 2 Oct 2018 19:49:10 - 1.75
+++ dev/usb/if_cdce.c 21 Jan 2019 05:42:47 -
@@ -103,6 +103,7 @@ const struct cdce_type cdce_devs[] = {
{{ USB_VENDOR_NETCHIP, USB_PRODUCT_NETCHIP_ETHERNETGADGET }, 0 },
{{ USB_VENDOR_COMPAQ, USB_PRODUCT_COMPAQ_IPAQLINUX }, 0 },
{{ USB_VENDOR_AMBIT, USB_PRODUCT_AMBIT_NTL_250 }, CDCE_SWAPUNION },
+{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E8372 }, 0 },
};
#define cdce_lookup(v, p) \
((const struct cdce_type *)usb_lookup(cdce_devs, v, p))
@@ -134,6 +135,18 @@ cdce_match(struct device *parent, void *
if (cdce_lookup(uaa->vendor, uaa->product) != NULL)
return (UMATCH_VENDOR_PRODUCT);
+
+ if (uaa->vendor == USB_VENDOR_HUAWEI &&
+ uaa->product == USB_PRODUCT_HUAWEI_E8372)
+ {
+ return (UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO);
+ }
+
+ if (uaa->vendor == USB_VENDOR_HUAWEI &&
+ uaa->product == USB_PRODUCT_HUAWEI_E8372)
+ {
+ return (UMATCH_IFACECLASS_IFACESUBCLASS_IFACEPROTO);
+ }
if (id->bInterfaceClass == UICLASS_CDC &&
(id->bInterfaceSubClass ==
Index: dev/usb/umsm.c
===
RCS file: /cvs/src/sys/dev/usb/umsm.c,v
retrieving revision 1.114
diff -u -p -u -p -r1.114 umsm.c
--- dev/usb/umsm.c 15 Aug 2018 14:13:07 - 1.114
+++ dev/usb/umsm.c 21 Jan 2019 05:42:48 -
@@ -133,7 +133,7 @@ static const struct umsm_type umsm_devs[
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E182 }, DEV_UMASS5},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E1820 }, DEV_UMASS5},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E220 }, DEV_HUAWEI},
- {{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E303 }, DEV_UMASS5},
+ {{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_ESERIES_INIT }, DEV_UMASS5},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E353_INIT }, DEV_UMASS5},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E510 }, DEV_HUAWEI},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E618 }, DEV_HUAWEI},
@@ -150,6 +150,7 @@ static const struct umsm_type umsm_devs[
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E1750 }, DEV_UMASS5},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E1752 }, 0},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E3372 }, DEV_UMASS5},
+ {{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E8372 }, DEV_UMASS5},
{{ USB_VENDOR_HYUNDAI, USB_PRODUCT_HYUNDAI_UM175 }, 0},
@@ -313,7 +314,10 @@ umsm_match(struct device *parent, void *
return UMATCH_VENDOR_IFACESUBCLASS;
else
return UMATCH_NONE;
- } else if (flag & DEV_UMASS) {
+ } else if (flag & DEV_UMASS5) {
+ return UMATCH_NONE;
+ }
+ else if (flag & DEV_UMASS) {
return UMATCH_VENDOR_IFACESUBCLASS;
}
Re: replacing timeout_add() with timeout_add_msec()
On Sat, Jan 12, 2019 at 10:40:35PM -0600, Amit Kulkarni wrote:
> > You started to convert the wrong timeout_add() calls. Doing a blind
> > timeout_add() to timeout_add_msec() conversion especially on calls with 0
> > or 1 tick sleep time is the wrong approach.
> > The right approach is to identify calls that do sleep for a well defined
> > time
> > (1sec, 50ms or whatever) and convert those. Most of the timeouts you
> > changes are not in that category. First I would look at timeouts that have
> > a multiplication with hz in them since those wait for a specific time.
>
> Thanks for the feedback Claudio and Mark!
>
> Here is a new diff based on your suggestions, looking for feedback, no tests
> requested yet. I assumed in below diff that 1hz == 1sec (confirmed in
> timeout_add_sec() in /sys/kern/kern_timeout.c), and converted some
> multiplication and some divisions, all related to hz. Did some conversions in
> comments also, because in future they might be used.
> -- amit
>
>
> diff --git sys/arch/armv7/exynos/exuart.c sys/arch/armv7/exynos/exuart.c
> index 4b0588750ea..15086bc5976 100644
> --- sys/arch/armv7/exynos/exuart.c
> +++ sys/arch/armv7/exynos/exuart.c
> @@ -283,7 +283,7 @@ exuart_intr(void *arg)
> if (p >= sc->sc_ibufend) {
> sc->sc_floods++;
> if (sc->sc_errors++ == 0)
> - timeout_add(>sc_diag_tmo, 60 * hz);
> + timeout_add_sec(>sc_diag_tmo, 60);
> } else {
> *p++ = c;
> if (p == sc->sc_ibufhigh && ISSET(tp->t_cflag, CRTSCTS))
> @@ -710,7 +710,7 @@ exuartclose(dev_t dev, int flag, int mode, struct proc *p)
> /* tty device is waiting for carrier; drop dtr then re-raise */
> //CLR(sc->sc_ucr3, EXUART_CR3_DSR);
> //bus_space_write_2(iot, ioh, EXUART_UCR3, sc->sc_ucr3);
> - timeout_add(>sc_dtr_tmo, hz * 2);
> + timeout_add_sec(>sc_dtr_tmo, 2);
> } else {
> /* no one else waiting; turn off the uart */
> exuart_pwroff(sc);
This one is OK.
> diff --git sys/arch/sparc64/dev/fd.c sys/arch/sparc64/dev/fd.c
> index 8d548062f83..654d8c95524 100644
> --- sys/arch/sparc64/dev/fd.c
> +++ sys/arch/sparc64/dev/fd.c
> @@ -1632,7 +1632,7 @@ loop:
> fdc->sc_state = RECALCOMPLETE;
> if (fdc->sc_flags & FDC_NEEDHEADSETTLE) {
> /* allow 1/30 second for heads to settle */
> - timeout_add(>fdcpseudointr_to, hz / 30);
> + timeout_add_msec(>fdcpseudointr_to, 33);
> return (1); /* will return later */
> }
>
Wonder if this should be 30 or 40 since 33 is rather odd.
> diff --git sys/dev/fdt/imxuart.c sys/dev/fdt/imxuart.c
> index 84c7eb5aee6..c2fd7e4a6d3 100644
> --- sys/dev/fdt/imxuart.c
> +++ sys/dev/fdt/imxuart.c
> @@ -228,7 +228,7 @@ imxuart_intr(void *arg)
> if (p >= sc->sc_ibufend) {
> sc->sc_floods++;
> if (sc->sc_errors++ == 0)
> - timeout_add(>sc_diag_tmo, 60 * hz);
> + timeout_add_sec(>sc_diag_tmo, 60);
> } else {
> *p++ = c;
> if (p == sc->sc_ibufhigh &&
> @@ -457,7 +457,7 @@ imxuart_softint(void *arg)
> if (ISSET(c, IMXUART_RX_OVERRUN)) {
> sc->sc_overflows++;
> if (sc->sc_errors++ == 0)
> - timeout_add(>sc_diag_tmo, 60 * hz);
> + timeout_add_sec(>sc_diag_tmo, 60);
> }
> /* This is ugly, but fast. */
>
> @@ -629,7 +629,7 @@ imxuartclose(dev_t dev, int flag, int mode, struct proc
> *p)
> /* tty device is waiting for carrier; drop dtr then re-raise */
> CLR(sc->sc_ucr3, IMXUART_CR3_DSR);
> bus_space_write_2(iot, ioh, IMXUART_UCR3, sc->sc_ucr3);
> - timeout_add(>sc_dtr_tmo, hz * 2);
> + timeout_add_sec(>sc_dtr_tmo, 2);
> } else {
> /* no one else waiting; turn off the uart */
> imxuart_pwroff(sc);
OK.
> diff --git sys/dev/ic/if_wi_hostap.c sys/dev/ic/if_wi_hostap.c
> index 64e3c10f3f5..155a391e7f9 100644
> --- sys/dev/ic/if_wi_hostap.c
> +++ sys/dev/ic/if_wi_hostap.c
> @@ -410,7 +410,7 @@ wihap_sta_timeout(void *v)
>
> /* Add wihap timeout if we have not already done so. */
> if (!timeout_pending(>tmo))
> - timeout_add(>tmo, hz / 10);
> + timeout_add_msec(>tmo, 100);
>
> splx(s);
> }
OK.
> diff --git sys/dev/ic/pluart.c sys/dev/ic/pluart.c
> index 0f024c0ad34..19bbb76f4a6 100644
> --- sys/dev/ic/pluart.c
> +++ sys/dev/ic/pluart.c
> @@ -225,7 +225,7 @@ pluart_intr(void *arg)
> if (p >= sc->sc_ibufend) {
>
Re: teach fstat about pfkey
On Mon, Jan 21, 2019 at 01:13:23PM +1000, David Gwynne wrote:
> i was trying to figure out which bit of ldpd had the pfkey socket (cos
> reading code is hard sometimes), but had to work a little bit too hard
> to figure it out from what fstat currently prints. this has fstat printf
> "pfkey" when it hits an AF_KEY socket, rather than hit the default
> handler which prints "30 raw 2 0x0".
>
> ok?
>
> Index: fstat.c
> ===
> RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 fstat.c
> --- fstat.c 16 Sep 2018 02:44:06 - 1.95
> +++ fstat.c 21 Jan 2019 03:11:13 -
> @@ -788,6 +788,10 @@ socktrans(struct kinfo_file *kf)
> printf(" %d ", kf->so_protocol);
> hide((void *)(uintptr_t)kf->f_data);
> break;
> + case AF_KEY:
> + printf("* pfkey");
Can you make this
printf("* pfkey %s", stype);
printf(" %d ", kf->so_protocol);
At least then it is consistent with route.
> + hide((void *)(uintptr_t)kf->f_data);
> + break;
> default:
> /* print protocol number and socket address */
> printf("* %d %s", kf->so_family, stype);
>
--
:wq Claudio
teach fstat about pfkey
i was trying to figure out which bit of ldpd had the pfkey socket (cos
reading code is hard sometimes), but had to work a little bit too hard
to figure it out from what fstat currently prints. this has fstat printf
"pfkey" when it hits an AF_KEY socket, rather than hit the default
handler which prints "30 raw 2 0x0".
ok?
Index: fstat.c
===
RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.95
diff -u -p -r1.95 fstat.c
--- fstat.c 16 Sep 2018 02:44:06 - 1.95
+++ fstat.c 21 Jan 2019 03:11:13 -
@@ -788,6 +788,10 @@ socktrans(struct kinfo_file *kf)
printf(" %d ", kf->so_protocol);
hide((void *)(uintptr_t)kf->f_data);
break;
+ case AF_KEY:
+ printf("* pfkey");
+ hide((void *)(uintptr_t)kf->f_data);
+ break;
default:
/* print protocol number and socket address */
printf("* %d %s", kf->so_family, stype);
Adapt to Allwinner device tree changes in linux >= 5.0-rc1
Adapt to allwinner device tree changes in linux >= 5.0-rc1
"allwinner,sun6i-a31-rtc" has been removed from h3/h5/r40/a64
507c6e89d6c4b2cd68a8e7ff69d1a00cf74b15dd
ARM: dts: sunxi: h3/h5: Fix up RTC device node and clock references
44ff3cafcd7f413e7710a58ac40cfdc3a9380097
arm64: dts: allwinner: a64: Fix up RTC device node and clock references
5f9e882825467105acafd208520b69bf95adb963
ARM: dts: sun8i: r40: Add RTC device node
compile tested only
Index: sxirtc.c
===
RCS file: /cvs/src/sys/dev/fdt/sxirtc.c,v
retrieving revision 1.2
diff -u -p -r1.2 sxirtc.c
--- sxirtc.c27 Mar 2017 14:03:19 - 1.2
+++ sxirtc.c21 Jan 2019 00:16:02 -
@@ -76,7 +76,9 @@ sxirtc_match(struct device *parent, void
return (OF_is_compatible(faa->fa_node, "allwinner,sun4i-a10-rtc") ||
OF_is_compatible(faa->fa_node, "allwinner,sun7i-a20-rtc") ||
- OF_is_compatible(faa->fa_node, "allwinner,sun6i-a31-rtc"));
+ OF_is_compatible(faa->fa_node, "allwinner,sun6i-a31-rtc") ||
+ OF_is_compatible(faa->fa_node, "allwinner,sun8i-h3-rtc") ||
+ OF_is_compatible(faa->fa_node, "allwinner,sun50i-h5-rtc"));
}
void
@@ -98,7 +100,9 @@ sxirtc_attach(struct device *parent, str
faa->fa_reg[0].size, 0, >sc_ioh))
panic("sxirtc_attach: bus_space_map failed!");
- if (OF_is_compatible(faa->fa_node, "allwinner,sun6i-a31-rtc")) {
+ if (OF_is_compatible(faa->fa_node, "allwinner,sun6i-a31-rtc") ||
+ OF_is_compatible(faa->fa_node, "allwinner,sun8i-h3-rtc") ||
+ OF_is_compatible(faa->fa_node, "allwinner,sun50i-h5-rtc")) {
sc->sc_yymmdd = SXIRTC_YYMMDD_A31;
sc->sc_hhmmss = SXIRTC_HHMMSS_A31;
} else {
ndp: zap unused ntop_buf
Last usage got removed in revision 1.9 date: 2001/02/08 08:35:17; author: itojun; state: Exp; lines: +109 -27; pull latest kame tree. ndp -n -a printing is now prettier with long IPv6 addresses. -l is deprecated (ignored). OK? Index: ndp.c === RCS file: /cvs/src/usr.sbin/ndp/ndp.c,v retrieving revision 1.90 diff -u -p -r1.90 ndp.c --- ndp.c 13 Jul 2018 09:03:44 - 1.90 +++ ndp.c 20 Jan 2019 22:41:24 - @@ -116,7 +116,6 @@ static int32_t thiszone;/* time differe static int rtsock = -1; static int repeat = 0; -char ntop_buf[INET6_ADDRSTRLEN]; /* inet_ntop() */ char host_buf[NI_MAXHOST]; /* getnameinfo() */ char ifix_buf[IFNAMSIZ]; /* if_indextoname() */
Re: arp timeouts and refresh arp entries before they expire
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.01.18 00:11:27 +0100:
> On Thu, Jan 17, 2019 at 03:21:58PM -0700, Theo de Raadt wrote:
> > - if (la_hold_total < LA_HOLD_TOTAL && la_hold_total < nmbclust / 64)
> > {
> > + if (la_hold_total < nmbclust / 64) {
> >
> > I have disagreed with claudio about this aspect of the diff.
> >
> > The refresh attempt is the crucial problem to fix, because of what it has
> > done to tcp with slowstart etc.
> >
> > If refresh works, we may not need to increase the hold mbufs count at all.
> >
> > Secondly, I don't like how this is coupled to the MD nmbclust value.
> > Some architectures have that very large, others very small, for other
> > reasons. Bringing it into play here isn't right.
>
> Updated diff that changes this limit back to LA_HOLD_TOTAL (100) but
> removes the la_hold_total < nmbclust / 64 check.
It works here.
Code reads ok fwiw.
/Benno
> --
> :wq Claudio
>
> Index: if_ether.c
> ===
> RCS file: /cvs/src/sys/netinet/if_ether.c,v
> retrieving revision 1.237
> diff -u -p -r1.237 if_ether.c
> --- if_ether.c30 Nov 2018 09:27:56 - 1.237
> +++ if_ether.c17 Jan 2019 22:59:59 -
> @@ -67,8 +67,9 @@
> struct llinfo_arp {
> LIST_ENTRY(llinfo_arp) la_list;
> struct rtentry *la_rt; /* backpointer to rtentry */
> - long la_asked; /* last time we QUERIED */
> struct mbuf_list la_ml; /* packet hold queue */
> + time_t la_refreshed; /* when was refresh sent */
> + int la_asked; /* number of queries sent */
> };
> #define LA_HOLD_QUEUE 10
> #define LA_HOLD_TOTAL 100
> @@ -119,7 +120,7 @@ arptimer(void *arg)
> LIST_FOREACH_SAFE(la, _list, la_list, nla) {
> struct rtentry *rt = la->la_rt;
>
> - if (rt->rt_expire && rt->rt_expire <= time_uptime)
> + if (rt->rt_expire && rt->rt_expire < time_uptime)
> arptfree(rt); /* timer has expired; clear */
> }
> NET_UNLOCK();
> @@ -130,7 +131,6 @@ arp_rtrequest(struct ifnet *ifp, int req
> {
> struct sockaddr *gate = rt->rt_gateway;
> struct llinfo_arp *la = (struct llinfo_arp *)rt->rt_llinfo;
> - struct ifaddr *ifa;
>
> if (!arpinit_done) {
> static struct timeout arptimer_to;
> @@ -140,7 +140,7 @@ arp_rtrequest(struct ifnet *ifp, int req
> IPL_SOFTNET, 0, "arp", NULL);
>
> timeout_set_proc(_to, arptimer, _to);
> - timeout_add_sec(_to, 1);
> + timeout_add_sec(_to, arpt_prune);
> }
>
> if (ISSET(rt->rt_flags, RTF_GATEWAY|RTF_BROADCAST|RTF_MULTICAST))
> @@ -149,17 +149,12 @@ arp_rtrequest(struct ifnet *ifp, int req
> switch (req) {
>
> case RTM_ADD:
> - if (rt->rt_flags & RTF_CLONING ||
> - ((rt->rt_flags & (RTF_LLINFO | RTF_LOCAL)) && !la)) {
> - /*
> - * Give this route an expiration time, even though
> - * it's a "permanent" route, so that routes cloned
> - * from it do not need their expiration time set.
> - */
> - rt->rt_expire = time_uptime;
> - if ((rt->rt_flags & RTF_CLONING) != 0)
> - break;
> + if (rt->rt_flags & RTF_CLONING) {
> + rt->rt_expire = 0;
> + break;
> }
> + if ((rt->rt_flags & RTF_LOCAL) && !la)
> + rt->rt_expire = 0;
> /*
>* Announce a new entry if requested or warn the user
>* if another station has this IP address.
> @@ -179,7 +174,7 @@ arp_rtrequest(struct ifnet *ifp, int req
> }
> satosdl(gate)->sdl_type = ifp->if_type;
> satosdl(gate)->sdl_index = ifp->if_index;
> - if (la != 0)
> + if (la != NULL)
> break; /* This happens on a route change */
> /*
>* Case 2: This route may come from cloning, or a manual route
> @@ -195,18 +190,9 @@ arp_rtrequest(struct ifnet *ifp, int req
> ml_init(>la_ml);
> la->la_rt = rt;
> rt->rt_flags |= RTF_LLINFO;
> + if ((rt->rt_flags & RTF_LOCAL) == 0)
> + rt->rt_expire = time_uptime;
> LIST_INSERT_HEAD(_list, la, la_list);
> -
> - TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) {
> - if ((ifa->ifa_addr->sa_family == AF_INET) &&
> - ifatoia(ifa)->ia_addr.sin_addr.s_addr ==
> - satosin(rt_key(rt))->sin_addr.s_addr)
> - break;
> - }
> - if (ifa) {
> -
Re: bgpctl neighbor group support
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.01.20 12:33:29 +0100:
> In many cases it is nice to be able to use a command against a group of
> neighbors (e.g. all exchange peers). This diff implements this for
> bgpctl neighbor group foo [clear|destroy|down|refresh|up]
> bgpctl show neighbor group foo [messages|terse|timers]
> bgpctl show rib neighbor group foo ...
> This should cover all cases.
>
> The bgpctl manpage probably needs some extra love.
the manpage part has whitspace problems: behind every "with" there
is a space.
one more comment below, otherwise ok benno@
> --
> :wq Claudio
>
> Index: bgpctl/bgpctl.8
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
> retrieving revision 1.84
> diff -u -p -r1.84 bgpctl.8
> --- bgpctl/bgpctl.8 12 Dec 2018 20:21:04 - 1.84
> +++ bgpctl/bgpctl.8 20 Jan 2019 11:26:54 -
> @@ -116,12 +116,14 @@ The
> .Ar reason
> cannot exceed 128 octets.
> .Ar peer
> -may be the neighbor's address or description.
> +may be the neighbor's address, description or a group description used with
> +.Cm group .
> .It Cm neighbor Ar peer Cm destroy
> Destroy a previously cloned peer.
> The peer must be down before calling this function.
> .Ar peer
> -may be the neighbor's address or description.
> +may be the neighbor's address, description or a group description used with
> +.Cm group .
> .It Cm neighbor Ar peer Cm down Op Ar reason
> Take the BGP session to the specified neighbor down.
> If a
> @@ -133,17 +135,20 @@ The
> .Ar reason
> cannot exceed 128 octets.
> .Ar peer
> -may be the neighbor's address or description.
> +may be the neighbor's address, description or a group description used with
> +.Cm group .
> .It Cm neighbor Ar peer Cm refresh
> Request the neighbor to re-send all routes.
> Note that the neighbor is not obliged to re-send all routes, or any routes at
> all, even if it announced the route refresh capability.
> .Ar peer
> -may be the neighbor's address or description.
> +may be the neighbor's address, description or a group description used with
> +.Cm group .
> .It Cm neighbor Ar peer Cm up
> Bring the BGP session to the specified neighbor up.
> .Ar peer
> -may be the neighbor's address or description.
> +may be the neighbor's address, description or a group description used with
> +.Cm group .
> .It Cm network add Ar prefix Op Ar arguments
> Add the specified prefix to the list of announced networks.
> It is possible to set various path attributes with additional
> @@ -283,7 +288,9 @@ Multiple options and filters can be used
> .It Cm show neighbor Ar peer modifier
> Show detailed information about the neighbor identified by
> .Ar peer ,
> -which may be the neighbor's address or description,
> +which may be the neighbor's address, description or a group description used
> +with
> +.Cm group ,
> according to the given
> .Ar modifier :
> .Pp
> @@ -339,6 +346,8 @@ Show all entries that are internal route
> Show RIB memory statistics.
> .It Cm neighbor Ar peer
> Show only entries from the specified peer.
> +.It Cm neighbor group Ar description
> +Show only entries from the specified peer group.
> .It Cm peer-as Ar as
> Show all entries with
> .Ar as
> Index: bgpctl/bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.227
> diff -u -p -r1.227 bgpctl.c
> --- bgpctl/bgpctl.c 19 Dec 2018 15:27:29 - 1.227
> +++ bgpctl/bgpctl.c 20 Jan 2019 05:22:03 -
> @@ -156,6 +156,7 @@ main(int argc, char *argv[])
>
> memcpy(, >peeraddr, sizeof(neighbor.addr));
> strlcpy(neighbor.descr, res->peerdesc, sizeof(neighbor.descr));
> + neighbor.is_group = res->is_group;
> strlcpy(neighbor.shutcomm, res->shutcomm, sizeof(neighbor.shutcomm));
>
> switch (res->action) {
> Index: bgpctl/parser.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
> retrieving revision 1.88
> diff -u -p -r1.88 parser.c
> --- bgpctl/parser.c 19 Dec 2018 15:27:29 - 1.88
> +++ bgpctl/parser.c 20 Jan 2019 05:20:23 -
> @@ -44,6 +44,7 @@ enum token_type {
> ASTYPE,
> PREFIX,
> PEERDESC,
> + GROUPDESC,
> RIBNAME,
> SHUTDOWN_COMMUNICATION,
> COMMUNITY,
> @@ -220,7 +221,13 @@ static const struct token t_show_mrt_fil
> { ENDTOKEN, "", NONE, NULL}
> };
>
> +static const struct token t_show_rib_neigh_group[] = {
> + { GROUPDESC,"", NONE, t_show_rib},
> + { ENDTOKEN, "", NONE, NULL}
> +};
> +
> static const struct token t_show_rib_neigh[] = {
> + { KEYWORD, "group",NONE, t_show_rib_neigh_group},
> { PEERADDRESS, "", NONE, t_show_rib},
> { PEERDESC, "", NONE, t_show_rib},
>
Re: what.1: remove BUGS section
On 1/20/19 3:11 PM, Ingo Schwarze wrote: > Hi Fabio, > > Fabio Scotoni wrote on Sun, Jan 20, 2019 at 12:52:18PM +0100: > >> The what(1) man page has a BUGS section, >> which mdoc(7) says is discouraged in OpenBSD. > > What? Using BUGS sections is perfectly fine. > > The mdoc(7) manual page does not intend to say that using BUGS > sections is discouraged in OpenBSD. > > Why do you think it says that? That's my mistake. I misinterpreted this part in mdoc(7): 160 \&.\e\(dq .Sh BUGS 161 \&.\e\(dq .Sh SECURITY CONSIDERATIONS 162 \&.\e\(dq Not used in OpenBSD. I mistook the list of section headings near the end to all fall through to SECURITY CONSIDERATIONS, which says "Not used in OpenBSD." I should've noticed that assessment being inaccurate because the STANDARDS section is clearly not discouraged and it's part of the same list. >> The contents of the BUGS section is mostly unhelpful anyway. > > Indeed, thanks for reporting the weird text. > It doesn't mention any BUGS, so it's in the wrong section. > >> Talking about BSD not being able to distribute SCCS doesn't help >> anyone; >> not behaving like the original SCCS is obvious considering the >> STANDARDS section specifically notes OpenBSD extensions and >> compliance with POSIX. > > I almost felt like proposing to delete the utility outright. > We stopped embedding identifier strings into binaries many years ago. >> Then again, who knows whether some scripts somewhere still call it. > Keeping it costs almost nothing, and it is not dangerous. what(1) *is* in an odd spot, being the only part of SCCS that actually got rewritten for BSD. CSSC and Schily SCCS would theoretically also provide what(1) and the rest of SCCS.
Re: what.1: remove BUGS section
Hi Ingo, On Sun, Jan 20, 2019 at 03:11:55PM +0100, Ingo Schwarze wrote: | > Talking about BSD not being able to distribute SCCS doesn't help | > anyone; | > not behaving like the original SCCS is obvious considering the | > STANDARDS section specifically notes OpenBSD extensions and | > compliance with POSIX. | | I almost felt like proposing to delete the utility outright. | We stopped embedding identifier strings into binaries many years ago. | | Then again, who knows whether some scripts somewhere still call it. | Keeping it costs almost nothing, and it is not dangerous. It's not completely useless: [weerd@pom] $ doas what /bsd /bsd OpenBSD 6.4-current (GENERIC.MP) #597: Thu Jan 10 21:47:25 MST 2019 [weerd@pom] $ what `which ksh` /bin/ksh PD KSH v5.2.14 99/07/13.2 Cheers, Paul 'WEiRD' de Weerd -- >[<++>-]<+++.>+++[<-->-]<.>+++[<+ +++>-]<.>++[<>-]<+.--.[-] http://www.weirdnet.nl/
Re: what.1: remove BUGS section
Hi Fabio, Fabio Scotoni wrote on Sun, Jan 20, 2019 at 12:52:18PM +0100: > The what(1) man page has a BUGS section, > which mdoc(7) says is discouraged in OpenBSD. What? Using BUGS sections is perfectly fine. The mdoc(7) manual page does not intend to say that using BUGS sections is discouraged in OpenBSD. Why do you think it says that? > The contents of the BUGS section is mostly unhelpful anyway. Indeed, thanks for reporting the weird text. It doesn't mention any BUGS, so it's in the wrong section. > Talking about BSD not being able to distribute SCCS doesn't help > anyone; > not behaving like the original SCCS is obvious considering the > STANDARDS section specifically notes OpenBSD extensions and > compliance with POSIX. I almost felt like proposing to delete the utility outright. We stopped embedding identifier strings into binaries many years ago. Then again, who knows whether some scripts somewhere still call it. Keeping it costs almost nothing, and it is not dangerous. So i only removed the BUGS section with the following commit. Yours, Ingo CVSROOT:/cvs Module name:src Changes by: schwa...@cvs.openbsd.org2019/01/20 07:03:19 Modified files: usr.bin/what : what.1 Log message: merge weird BUGS section into HISTORY; issue reported by Fabio Scotoni Index: what.1 === RCS file: /cvs/src/usr.bin/what/what.1,v retrieving revision 1.19 diff -u -r1.19 what.1 --- what.1 22 Jan 2015 19:10:17 - 1.19 +++ what.1 20 Jan 2019 14:00:44 - @@ -92,16 +92,6 @@ .Sh HISTORY The .Nm -command appeared in -.Bx 4.0 . -.Sh BUGS -As -.Bx -is not licensed to distribute -.Tn SCCS , -this is a rewrite of the -.Nm -command which is part of -.Tn SCCS . -As such it may not behave exactly the same as that -command does. +command first appeared in the SCCS package and was rewritten for +.Bx 4.0 +for licensing reasons.
Re: if_pppoe.c patch
On Sun, Jan 20, 2019 at 12:56:22PM +, Stuart Henderson wrote: > On 2019/01/18 10:59, Peter J. Philipp wrote: > > I have "covered" up PPPoE Session ID's from users because it is a value that > > is only gotten on the Data Link layer and historically non-root users did > > not > > have access to that. It really is a value that doesn't concern them. I > > have > > wrapped the display with a suser() conditional. The magic value 0x is > > not used/reserved according to RFC 2516. > > No real comment on whether we should do this or not (it feels a bit like > restricting arp to root..) but if it is done, it would seem better to use Not like restricting arp to root..., it's more like TCP hiding its ISN. > a value which cannot be sent on the wire, rather than one which is just > reserved. (And actually hide it from ifconfig rather than displaying a > bogus value). I'll try to get around to it tomorrow if I can. Otherwise just drop the request. :-) Best Regards, -peter > > as root: > > > > beta# ifconfig pppoe0 > > pppoe0: flags=8810 mtu 1492 > > index 12 priority 0 llprio 3 > > dev: state: initial > > sid: 0x0 PADI retries: 0 PADR retries: 0 > > groups: pppoe > > > > as non-root: > > > > beta$ ifconfig pppoe0 > > pppoe0: flags=8810 mtu 1492 > > index 12 priority 0 llprio 3 > > dev: state: initial > > sid: 0x PADI retries: 0 PADR retries: 0 > > groups: pppoe > > > > > > patch follows: > > > > Index: if_pppoe.c > > === > > RCS file: /cvs/src/sys/net/if_pppoe.c,v > > retrieving revision 1.67 > > diff -u -p -u -r1.67 if_pppoe.c > > --- if_pppoe.c 19 Feb 2018 08:59:52 - 1.67 > > +++ if_pppoe.c 18 Jan 2019 09:50:58 - > > @@ -874,7 +876,12 @@ pppoe_ioctl(struct ifnet *ifp, unsigned > > struct pppoeconnectionstate *state = > > (struct pppoeconnectionstate *)data; > > state->state = sc->sc_state; > > - state->session_id = sc->sc_session; > > + > > + if (! suser(p)) > > + state->session_id = sc->sc_session; > > + else > > + state->session_id = 0x; /* reserved sid */ > > + > > state->padi_retry_no = sc->sc_padi_retried; > > state->padr_retry_no = sc->sc_padr_retried; > > state->session_time.tv_sec = sc->sc_session_time.tv_sec; > >
what.1: remove BUGS section
The what(1) man page has a BUGS section, which mdoc(7) says is discouraged in OpenBSD. The contents of the BUGS section is mostly unhelpful anyway. Talking about BSD not being able to distribute SCCS doesn't help anyone; not behaving like the original SCCS is obvious considering the STANDARDS section specifically notes OpenBSD extensions and compliance with POSIX. Index: usr.bin/what/what.1 === RCS file: /cvs/src/usr.bin/what/what.1,v retrieving revision 1.19 diff -u -p -r1.19 what.1 --- usr.bin/what/what.1 22 Jan 2015 19:10:17 - 1.19 +++ usr.bin/what/what.1 20 Jan 2019 10:04:41 - @@ -94,14 +94,3 @@ The .Nm command appeared in .Bx 4.0 . -.Sh BUGS -As -.Bx -is not licensed to distribute -.Tn SCCS , -this is a rewrite of the -.Nm -command which is part of -.Tn SCCS . -As such it may not behave exactly the same as that -command does.
Re: if_pppoe.c patch
On 2019/01/18 10:59, Peter J. Philipp wrote: > I have "covered" up PPPoE Session ID's from users because it is a value that > is only gotten on the Data Link layer and historically non-root users did not > have access to that. It really is a value that doesn't concern them. I have > wrapped the display with a suser() conditional. The magic value 0x is > not used/reserved according to RFC 2516. No real comment on whether we should do this or not (it feels a bit like restricting arp to root..) but if it is done, it would seem better to use a value which cannot be sent on the wire, rather than one which is just reserved. (And actually hide it from ifconfig rather than displaying a bogus value). > as root: > > beta# ifconfig pppoe0 > pppoe0: flags=8810 mtu 1492 > index 12 priority 0 llprio 3 > dev: state: initial > sid: 0x0 PADI retries: 0 PADR retries: 0 > groups: pppoe > > as non-root: > > beta$ ifconfig pppoe0 > pppoe0: flags=8810 mtu 1492 > index 12 priority 0 llprio 3 > dev: state: initial > sid: 0x PADI retries: 0 PADR retries: 0 > groups: pppoe > > > patch follows: > > Index: if_pppoe.c > === > RCS file: /cvs/src/sys/net/if_pppoe.c,v > retrieving revision 1.67 > diff -u -p -u -r1.67 if_pppoe.c > --- if_pppoe.c19 Feb 2018 08:59:52 - 1.67 > +++ if_pppoe.c18 Jan 2019 09:50:58 - > @@ -874,7 +876,12 @@ pppoe_ioctl(struct ifnet *ifp, unsigned > struct pppoeconnectionstate *state = > (struct pppoeconnectionstate *)data; > state->state = sc->sc_state; > - state->session_id = sc->sc_session; > + > + if (! suser(p)) > + state->session_id = sc->sc_session; > + else > + state->session_id = 0x; /* reserved sid */ > + > state->padi_retry_no = sc->sc_padi_retried; > state->padr_retry_no = sc->sc_padr_retried; > state->session_time.tv_sec = sc->sc_session_time.tv_sec; >
Re: bgpctl neighbor group support
On 2019/01/20 12:33, Claudio Jeker wrote: > In many cases it is nice to be able to use a command against a group of > neighbors (e.g. all exchange peers). This diff implements this for > bgpctl neighbor group foo [clear|destroy|down|refresh|up] > bgpctl show neighbor group foo [messages|terse|timers] > bgpctl show rib neighbor group foo ... > This should cover all cases. This is *very* useful, thanks. Works/reads fine here - OK sthen@ > The bgpctl manpage probably needs some extra love. > -may be the neighbor's address or description. > +may be the neighbor's address, description or a group description used with > +.Cm group . Possible alternative, it's slightly lengthier but I find it a bit easier reading. Index: bgpctl.8 === RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v retrieving revision 1.84 diff -u -p -r1.84 bgpctl.8 --- bgpctl.812 Dec 2018 20:21:04 - 1.84 +++ bgpctl.820 Jan 2019 11:56:34 - @@ -116,12 +116,16 @@ The .Ar reason cannot exceed 128 octets. .Ar peer -may be the neighbor's address or description. +may be the neighbor's address, description or the word +.Cm group +followed by a group description. .It Cm neighbor Ar peer Cm destroy Destroy a previously cloned peer. The peer must be down before calling this function. .Ar peer -may be the neighbor's address or description. +may be the neighbor's address, description or the word +.Cm group +followed by a group description. .It Cm neighbor Ar peer Cm down Op Ar reason Take the BGP session to the specified neighbor down. If a @@ -133,17 +137,23 @@ The .Ar reason cannot exceed 128 octets. .Ar peer -may be the neighbor's address or description. +may be the neighbor's address, description or the word +.Cm group +followed by a group description. .It Cm neighbor Ar peer Cm refresh Request the neighbor to re-send all routes. Note that the neighbor is not obliged to re-send all routes, or any routes at all, even if it announced the route refresh capability. .Ar peer -may be the neighbor's address or description. +may be the neighbor's address, description or the word +.Cm group +followed by a group description. .It Cm neighbor Ar peer Cm up Bring the BGP session to the specified neighbor up. .Ar peer -may be the neighbor's address or description. +may be the neighbor's address, description or the word +.Cm group +followed by a group description. .It Cm network add Ar prefix Op Ar arguments Add the specified prefix to the list of announced networks. It is possible to set various path attributes with additional @@ -283,7 +293,6 @@ Multiple options and filters can be used .It Cm show neighbor Ar peer modifier Show detailed information about the neighbor identified by .Ar peer , -which may be the neighbor's address or description, according to the given .Ar modifier : .Pp @@ -299,6 +308,10 @@ prefix count, the number of sent and rec .It Cm timers Show the BGP timers. .El +.Ar peer +may be the neighbor's address, description or the word +.Cm group +followed by a group description. .It Cm show nexthop Show the list of BGP nexthops and the result of their validity check. .It Xo @@ -339,6 +352,8 @@ Show all entries that are internal route Show RIB memory statistics. .It Cm neighbor Ar peer Show only entries from the specified peer. +.It Cm neighbor group Ar description +Show only entries from the specified peer group. .It Cm peer-as Ar as Show all entries with .Ar as
bgpctl neighbor group support
In many cases it is nice to be able to use a command against a group of
neighbors (e.g. all exchange peers). This diff implements this for
bgpctl neighbor group foo [clear|destroy|down|refresh|up]
bgpctl show neighbor group foo [messages|terse|timers]
bgpctl show rib neighbor group foo ...
This should cover all cases.
The bgpctl manpage probably needs some extra love.
--
:wq Claudio
Index: bgpctl/bgpctl.8
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
retrieving revision 1.84
diff -u -p -r1.84 bgpctl.8
--- bgpctl/bgpctl.8 12 Dec 2018 20:21:04 - 1.84
+++ bgpctl/bgpctl.8 20 Jan 2019 11:26:54 -
@@ -116,12 +116,14 @@ The
.Ar reason
cannot exceed 128 octets.
.Ar peer
-may be the neighbor's address or description.
+may be the neighbor's address, description or a group description used with
+.Cm group .
.It Cm neighbor Ar peer Cm destroy
Destroy a previously cloned peer.
The peer must be down before calling this function.
.Ar peer
-may be the neighbor's address or description.
+may be the neighbor's address, description or a group description used with
+.Cm group .
.It Cm neighbor Ar peer Cm down Op Ar reason
Take the BGP session to the specified neighbor down.
If a
@@ -133,17 +135,20 @@ The
.Ar reason
cannot exceed 128 octets.
.Ar peer
-may be the neighbor's address or description.
+may be the neighbor's address, description or a group description used with
+.Cm group .
.It Cm neighbor Ar peer Cm refresh
Request the neighbor to re-send all routes.
Note that the neighbor is not obliged to re-send all routes, or any routes at
all, even if it announced the route refresh capability.
.Ar peer
-may be the neighbor's address or description.
+may be the neighbor's address, description or a group description used with
+.Cm group .
.It Cm neighbor Ar peer Cm up
Bring the BGP session to the specified neighbor up.
.Ar peer
-may be the neighbor's address or description.
+may be the neighbor's address, description or a group description used with
+.Cm group .
.It Cm network add Ar prefix Op Ar arguments
Add the specified prefix to the list of announced networks.
It is possible to set various path attributes with additional
@@ -283,7 +288,9 @@ Multiple options and filters can be used
.It Cm show neighbor Ar peer modifier
Show detailed information about the neighbor identified by
.Ar peer ,
-which may be the neighbor's address or description,
+which may be the neighbor's address, description or a group description used
+with
+.Cm group ,
according to the given
.Ar modifier :
.Pp
@@ -339,6 +346,8 @@ Show all entries that are internal route
Show RIB memory statistics.
.It Cm neighbor Ar peer
Show only entries from the specified peer.
+.It Cm neighbor group Ar description
+Show only entries from the specified peer group.
.It Cm peer-as Ar as
Show all entries with
.Ar as
Index: bgpctl/bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.227
diff -u -p -r1.227 bgpctl.c
--- bgpctl/bgpctl.c 19 Dec 2018 15:27:29 - 1.227
+++ bgpctl/bgpctl.c 20 Jan 2019 05:22:03 -
@@ -156,6 +156,7 @@ main(int argc, char *argv[])
memcpy(, >peeraddr, sizeof(neighbor.addr));
strlcpy(neighbor.descr, res->peerdesc, sizeof(neighbor.descr));
+ neighbor.is_group = res->is_group;
strlcpy(neighbor.shutcomm, res->shutcomm, sizeof(neighbor.shutcomm));
switch (res->action) {
Index: bgpctl/parser.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
retrieving revision 1.88
diff -u -p -r1.88 parser.c
--- bgpctl/parser.c 19 Dec 2018 15:27:29 - 1.88
+++ bgpctl/parser.c 20 Jan 2019 05:20:23 -
@@ -44,6 +44,7 @@ enum token_type {
ASTYPE,
PREFIX,
PEERDESC,
+ GROUPDESC,
RIBNAME,
SHUTDOWN_COMMUNICATION,
COMMUNITY,
@@ -220,7 +221,13 @@ static const struct token t_show_mrt_fil
{ ENDTOKEN, "", NONE, NULL}
};
+static const struct token t_show_rib_neigh_group[] = {
+ { GROUPDESC,"", NONE, t_show_rib},
+ { ENDTOKEN, "", NONE, NULL}
+};
+
static const struct token t_show_rib_neigh[] = {
+ { KEYWORD, "group",NONE, t_show_rib_neigh_group},
{ PEERADDRESS, "", NONE, t_show_rib},
{ PEERDESC, "", NONE, t_show_rib},
{ ENDTOKEN, "", NONE, NULL}
@@ -236,13 +243,6 @@ static const struct token t_show_rib_rib
{ ENDTOKEN, "", NONE, NULL}
};
-static const struct token t_show_neighbor[] = {
- { NOTOKEN, "", NONE, NULL},
- { PEERADDRESS, "", NONE, t_show_neighbor_modifiers},
- { PEERDESC, "",
