pax: GNU tar base-256 size field support
I recently ran into a problem with busybox tar generating archives
where the size field is base-256 encoded for files larger than 8GB.
Apparently this is a GNU tar extension.
Do we want to support this in pax? Below is an initial diff that
at least produces the correct results when listing the archive. I
have not yet verified that it gets extracted correctly. If there
is interest I will do some more testing.
- todd
Index: bin/pax/gen_subs.c
===
RCS file: /cvs/src/bin/pax/gen_subs.c,v
retrieving revision 1.32
diff -u -p -u -r1.32 gen_subs.c
--- bin/pax/gen_subs.c 26 Aug 2016 05:06:14 - 1.32
+++ bin/pax/gen_subs.c 19 Apr 2023 02:05:19 -
@@ -45,6 +45,7 @@
#include
#include
#include
+#include
#include "pax.h"
#include "extern.h"
@@ -279,9 +280,10 @@ ul_asc(u_long val, char *str, int len, i
/*
* asc_ull()
- * Convert hex/octal character string into a unsigned long long.
- * We do not have to check for overflow! (The headers in all
- * supported formats are not large enough to create an overflow).
+ * Convert hex/octal/base-256 character string into a unsigned long long.
+ * We only have to check for overflow when parsing base-256.
+ * The headers in all supported formats are not large enough to create
+ * an overflow for hex or octal.
* NOTE: strings passed to us are NOT TERMINATED.
* Return:
* unsigned long long value
@@ -296,9 +298,9 @@ asc_ull(char *str, int len, int base)
stop = str + len;
/*
-* skip over leading blanks and zeros
+* skip over leading blanks
*/
- while ((str < stop) && ((*str == ' ') || (*str == '0')))
+ while ((str < stop) && (*str == ' '))
++str;
/*
@@ -316,7 +318,17 @@ asc_ull(char *str, int len, int base)
else
break;
}
+ } else if (*str == '\200') {
+ /* base-256 encoding, GNU tar extension */
+ while (++str < stop) {
+ if (tval + (unsigned char)*str > ULLONG_MAX >> 8) {
+ /* overflow */
+ return(-1);
+ }
+ tval = (tval << 8) + (unsigned char)*str;
+ }
} else {
+ /* octal */
while ((str < stop) && (*str >= '0') && (*str <= '7'))
tval = (tval << 3) + (*str++ - '0');
}
Fwd: [patch] [arm64] cpu.c patch based on amd64 idea, provides more debug for multicore kernel
Hello, tech! I'm attaching simple patch that adds two "features" to mp kernel for arm64 arch: 1. It allows to enable debug flag to drop to DDB to debug mp init problems 2. It allows some buggy arm64 processors to break from loop and dirty (or not so dirty) boot to multicore instead of eternal hang in while. This based on same idea as amd64 cpu.c function - instead of waiting for while() to boot secondary core either continue to try boot next one or drop to DDB. -- Nerfur Dragon -==(UDIC)==- arm64_cpu.c.diff Description: Binary data
Re: in_ioctl*: hoist identical privilege checks
On Sat, Apr 15, 2023 at 01:48:02PM +, Klemens Nanni wrote:
> On Fri, Apr 14, 2023 at 11:33:18PM +, Klemens Nanni wrote:
> > All cases do the same check up first, so merge it before the switch.
Committed.
> > It could be hoisted further in both in_ioctl() and in_ioctl_change_ifaddr(),
> > but that meant a change in errno return semantic, so leave it for now.
>
> in6.c already has the privilege check as early as possible, so here's a diff
> that simplifies in.c to match in6.c.
>
> I can't think of a scenario where returning EPERM (this diff) instead of
> whatever errno the currently earlier sanity checks yield would break.
>
> It is an unprivileged ioctl call in the first place, so EPERM as soon as
> possible makes sense to me.
Better and rebased diff, no locks taken for unprivileged calls that *will*
fail, EPERM is returned first in IPv4 like IPv6 already does, nicely syncing
the ioctl handlers.
Feedback? OK?
Index: netinet6/in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.259
diff -u -p -r1.259 in6.c
--- netinet6/in6.c 6 Dec 2022 22:19:39 - 1.259
+++ netinet6/in6.c 18 Apr 2023 22:40:40 -
@@ -196,10 +196,9 @@ in6_sa2sin6(struct sockaddr *sa, struct
int
in6_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp)
{
- int privileged;
+ int privileged = 0;
int error;
- privileged = 0;
if ((so->so_state & SS_PRIV) != 0)
privileged++;
Index: netinet/in.c
===
RCS file: /cvs/src/sys/netinet/in.c,v
retrieving revision 1.181
diff -u -p -r1.181 in.c
--- netinet/in.c18 Apr 2023 22:20:16 - 1.181
+++ netinet/in.c18 Apr 2023 22:40:41 -
@@ -84,8 +84,8 @@
void in_socktrim(struct sockaddr_in *);
-int in_ioctl_set_ifaddr(u_long, caddr_t, struct ifnet *, int);
-int in_ioctl_change_ifaddr(u_long, caddr_t, struct ifnet *, int);
+int in_ioctl_set_ifaddr(u_long, caddr_t, struct ifnet *);
+int in_ioctl_change_ifaddr(u_long, caddr_t, struct ifnet *);
int in_ioctl_get(u_long, caddr_t, struct ifnet *);
void in_purgeaddr(struct ifaddr *);
int in_addhost(struct in_ifaddr *, struct sockaddr_in *);
@@ -199,10 +199,9 @@ in_sa2sin(struct sockaddr *sa, struct so
int
in_control(struct socket *so, u_long cmd, caddr_t data, struct ifnet *ifp)
{
- int privileged;
+ int privileged = 0;
int error;
- privileged = 0;
if ((so->so_state & SS_PRIV) != 0)
privileged++;
@@ -242,10 +241,14 @@ in_ioctl(u_long cmd, caddr_t data, struc
case SIOCGIFBRDADDR:
return in_ioctl_get(cmd, data, ifp);
case SIOCSIFADDR:
- return in_ioctl_set_ifaddr(cmd, data, ifp, privileged);
+ if (!privileged)
+ return (EPERM);
+ return in_ioctl_set_ifaddr(cmd, data, ifp);
case SIOCAIFADDR:
case SIOCDIFADDR:
- return in_ioctl_change_ifaddr(cmd, data, ifp, privileged);
+ if (!privileged)
+ return (EPERM);
+ return in_ioctl_change_ifaddr(cmd, data, ifp);
case SIOCSIFNETMASK:
case SIOCSIFDSTADDR:
case SIOCSIFBRDADDR:
@@ -254,6 +257,9 @@ in_ioctl(u_long cmd, caddr_t data, struc
return (EOPNOTSUPP);
}
+ if (!privileged)
+ return (EPERM);
+
if (ifr->ifr_addr.sa_family == AF_INET) {
error = in_sa2sin(>ifr_addr, );
if (error)
@@ -282,11 +288,6 @@ in_ioctl(u_long cmd, caddr_t data, struc
goto err;
}
- if (!privileged) {
- error = EPERM;
- goto err;
- }
-
switch (cmd) {
case SIOCSIFDSTADDR:
if ((ifp->if_flags & IFF_POINTOPOINT) == 0) {
@@ -342,8 +343,7 @@ err:
}
int
-in_ioctl_set_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp,
-int privileged)
+in_ioctl_set_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
{
struct ifreq *ifr = (struct ifreq *)data;
struct ifaddr *ifa;
@@ -355,9 +355,6 @@ in_ioctl_set_ifaddr(u_long cmd, caddr_t
if (cmd != SIOCSIFADDR)
panic("%s: invalid ioctl %lu", __func__, cmd);
- if (!privileged)
- return (EPERM);
-
error = in_sa2sin(>ifr_addr, );
if (error)
return (error);
@@ -402,8 +399,7 @@ in_ioctl_set_ifaddr(u_long cmd, caddr_t
}
int
-in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp,
-int privileged)
+in_ioctl_change_ifaddr(u_long cmd, caddr_t data, struct ifnet *ifp)
{
struct ifaddr *ifa;
struct in_ifaddr *ia = NULL;
@@ -418,9 +414,6 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
if (error)
return (error);
}
-
- if (!privileged)
-
synaptics touchpad with no multifinger support
Hello, tech@ So I installed OpenBSD on old Compaq laptop and noticed that scrolling with touchpad is not working. I started investigating to see why it is the case and found out a few odd things in wscons(4) and pckbc(4) drivers. Basically, the laptop's touchpad lacks multifinger support (see dmesg below), but nevertheless, wscons(4) sets it to two-finger mode, which doesn't work, obviously. Initially, I thought about adding something like mouse.tp.edgescroll to wsconsctl(8). I then found out that there is already undocumented mouse.tp.param option, that can be used to force the edgescroll mode by setting 67 (WSMOUSECFG_TWOFINGERSCROLL) to 0 and 68 (WSMOUSECFG_EDGESCROLL) to 1, but I don't think it is very convenient and readable. Later I continued to investigate, why wscons(4) defaults to two-finger mode and found that it does so by examining a field of a touchpad data structure called 'contacts_max' [1], which, I assumed, represents a number of maximum simultaneous touches. But when I grepped for this name to see where this value is set, I found that it is set in pckbc(4). It is done in a very simple way, so it sets it to 3 (well, actually SYNAPTICS_MAX_FINGERS, but it is defined to be 3 only there and nowhere else) if the device in question is a synaptics touchpad (!) [2], even though the touchpad has a capabilities field and it is recognized correctly (SYNAPTICS_CAP_MULTIFINGER is set to 0 in my case). So I think a solution would be at least checking SYNAPTIC_CAP_MULTIFINGER and setting contacts_max depending on that (3 if true, 1 if false). Maybe it is a bodge, but it works. Index: pms.c === RCS file: /cvs/src/sys/dev/pckbc/pms.c,v retrieving revision 1.97 diff -u -p -r1.97 pms.c --- pms.c 23 Jul 2022 05:55:16 - 1.97 +++ pms.c 18 Apr 2023 21:36:10 - @@ -1075,7 +1075,10 @@ synaptics_get_hwinfo(struct pms_softc *s hw->y_max = (max_coords ? SYNAPTICS_Y_LIMIT(max_coords) : SYNAPTICS_YMAX_BEZEL); - hw->contacts_max = SYNAPTICS_MAX_FINGERS; + if(syn->capabilities & SYNAPTICS_CAP_MULTIFINGER) + hw->contacts_max = SYNAPTICS_MAX_FINGERS; + else + hw->contacts_max = 1; syn->sec_buttons = 0; [1]: /sys/dev/wscons/wstpad.c:1573 [2]: /sys/dev/pckbc/psm.c:1078 Here are relevant lines from dmesg(8): pms0 at pckbc0 (aux slot) wsmouse0 at pms0 mux 0 pms0: Synaptics touchpad, firmware 6.5, 0x1c0b1 0xa0 0x0 0xa04751 0x0 So I'm looking forward for comments on this. Has anybody experienced something similar? -- best regards, la ninpre.
Re: bgpd pass flowspec rules to RDE
On Tue, Apr 18, 2023 at 05:22:26PM +0200, Claudio Jeker wrote: > This adds the needed bits to send the flowspec rules to the RDE. > The RDE just drops them on the ground for now. ok tb
Re: bgpd add flowspec support to rde_prefix.c
On Tue, Apr 18, 2023 at 03:38:12PM +0200, Claudio Jeker wrote:
> Extend the pt_entry api to handle flowspec.
> Introduce pt_get_flow() and pt_add_flow() to lookup and insert flowspec
> objects. Add pt_getflowspec() which works somewhat similar to pt_getaddr()
> to extract the flowspec NLRI from a pt_entry.
>
> There is a hack in pt_getaddr() to return something. This is the
> destination prefix of the flowspec rule (or a default route if that is
> missing). Also extend pt_write() to handle flowspec NLRI.
This reads fine to me. These casting and union gymnastics make me squirm,
but it's not the moment to rethink this.
Regarding the unused prefixlen, I think that's only superficially the
case. flowspec_cmp() will extract it again and use it.
So I think this is ok as it is.
> @@ -471,7 +575,8 @@ pt_writebuf(struct ibuf *buf, struct pt_
> {
> struct pt_entry_vpn4*pvpn4 = (struct pt_entry_vpn4 *)pte;
> struct pt_entry_vpn6*pvpn6 = (struct pt_entry_vpn6 *)pte;
> - int totlen;
> + struct pt_entry_flow*pflow = (struct pt_entry_flow *)pte;
> + int totlen, flowlen;
^
There is a space before a tab hiding here.
Re: Call sysctl_ifnames() with shared netlock
On Mon, Apr 17, 2023 at 05:43:05PM +0300, Vitaliy Makkoveev wrote:
> On Mon, Apr 17, 2023 at 02:58:59PM +0200, Alexander Bluhm wrote:
> > On Mon, Apr 17, 2023 at 01:20:28AM +0300, Vitaliy Makkoveev wrote:
> > > It performs read-only access to netlock protected data.
> >
> > OK bluhm@
> >
> > Could you somewhere document that ifnetlist is protected by netlock?
> >
>
> We use both kernel and net lock for protect `ifnetlist'. This is the
> exception, so I propose to document this like below.
OK bluhm@
> Index: sys/net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.688
> diff -u -p -r1.688 if.c
> --- sys/net/if.c 8 Apr 2023 13:49:38 - 1.688
> +++ sys/net/if.c 17 Apr 2023 14:42:10 -
> @@ -272,6 +272,10 @@ ifinit(void)
>
> static struct if_idxmap if_idxmap;
>
> +/*
> + * XXXSMP: For `ifnetlist' modification both kernel and net locks
> + * should be taken. For read-only access only one lock of them required.
> + */
> struct ifnet_head ifnetlist = TAILQ_HEAD_INITIALIZER(ifnetlist);
>
> static inline unsigned int
> Index: sys/net/if_var.h
> ===
> RCS file: /cvs/src/sys/net/if_var.h,v
> retrieving revision 1.123
> diff -u -p -r1.123 if_var.h
> --- sys/net/if_var.h 5 Apr 2023 19:35:23 - 1.123
> +++ sys/net/if_var.h 17 Apr 2023 14:42:10 -
> @@ -121,7 +121,7 @@ TAILQ_HEAD(ifnet_head, ifnet);/* the a
> struct ifnet { /* and the entries */
> void*if_softc; /* [I] lower-level data for this if */
> struct refcnt if_refcnt;
> - TAILQ_ENTRY(ifnet) if_list; /* [K] all struct ifnets are chained */
> + TAILQ_ENTRY(ifnet) if_list; /* [NK] all struct ifnets are chained */
> TAILQ_HEAD(, ifaddr) if_addrlist; /* [N] list of addresses per if */
> TAILQ_HEAD(, ifmaddr) if_maddrlist; /* [N] list of multicast records */
> TAILQ_HEAD(, ifg_list) if_groups; /* [N] list of groups per if */
Re: Remove kernel lock from ifa_ifwithaddr()
On Tue, Apr 18, 2023 at 06:49:39PM +0300, Vitaliy Makkoveev wrote:
> rtable_setsource(... , NULL) actually doesn't destroy object pointed by
> `ar_source', so for this call netlock is not required. However this
> optimisation doesn't produce any visible effect, so no objections to
> call rt_setsource() with netlock held.
>
> ok?
OK bluhm@
> Index: sys/dev/usb/if_umb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 if_umb.c
> --- sys/dev/usb/if_umb.c 31 Mar 2023 23:53:49 - 1.50
> +++ sys/dev/usb/if_umb.c 18 Apr 2023 15:45:04 -
> @@ -1829,6 +1829,7 @@ umb_add_inet_config(struct umb_softc *sc
> default_sin.sin_len = sizeof (default_sin);
>
> memset(, 0, sizeof(info));
> + NET_LOCK();
> info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> info.rti_ifa = ifa_ifwithaddr(sintosa(_addr),
> ifp->if_rdomain);
> @@ -1836,7 +1837,6 @@ umb_add_inet_config(struct umb_softc *sc
> info.rti_info[RTAX_NETMASK] = sintosa(_sin);
> info.rti_info[RTAX_GATEWAY] = sintosa(_dstaddr);
>
> - NET_LOCK();
> rv = rtrequest(RTM_ADD, , 0, , ifp->if_rdomain);
> NET_UNLOCK();
> if (rv) {
> @@ -1910,6 +1910,7 @@ umb_add_inet6_config(struct umb_softc *s
> default_sin6.sin6_len = sizeof (default_sin6);
>
> memset(, 0, sizeof(info));
> + NET_LOCK();
> info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> info.rti_ifa = ifa_ifwithaddr(sin6tosa(_addr),
> ifp->if_rdomain);
> @@ -1917,7 +1918,6 @@ umb_add_inet6_config(struct umb_softc *s
> info.rti_info[RTAX_NETMASK] = sin6tosa(_sin6);
> info.rti_info[RTAX_GATEWAY] = sin6tosa(_dstaddr);
>
> - NET_LOCK();
> rv = rtrequest(RTM_ADD, , 0, , ifp->if_rdomain);
> NET_UNLOCK();
> if (rv) {
> Index: sys/net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.688
> diff -u -p -r1.688 if.c
> --- sys/net/if.c 8 Apr 2023 13:49:38 - 1.688
> +++ sys/net/if.c 18 Apr 2023 15:45:04 -
> @@ -1409,8 +1409,9 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
> struct ifaddr *ifa;
> u_int rdomain;
>
> + NET_ASSERT_LOCKED();
> +
> rdomain = rtable_l2(rtableid);
> - KERNEL_LOCK();
> TAILQ_FOREACH(ifp, , if_list) {
> if (ifp->if_rdomain != rdomain)
> continue;
> @@ -1420,12 +1421,10 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
> continue;
>
> if (equal(addr, ifa->ifa_addr)) {
> - KERNEL_UNLOCK();
> return (ifa);
> }
> }
> }
> - KERNEL_UNLOCK();
> return (NULL);
> }
>
> @@ -1438,8 +1437,9 @@ ifa_ifwithdstaddr(struct sockaddr *addr,
> struct ifnet *ifp;
> struct ifaddr *ifa;
>
> + NET_ASSERT_LOCKED();
> +
> rdomain = rtable_l2(rdomain);
> - KERNEL_LOCK();
> TAILQ_FOREACH(ifp, , if_list) {
> if (ifp->if_rdomain != rdomain)
> continue;
> @@ -1449,13 +1449,11 @@ ifa_ifwithdstaddr(struct sockaddr *addr,
> addr->sa_family || ifa->ifa_dstaddr == NULL)
> continue;
> if (equal(addr, ifa->ifa_dstaddr)) {
> - KERNEL_UNLOCK();
> return (ifa);
> }
> }
> }
> }
> - KERNEL_UNLOCK();
> return (NULL);
> }
>
> @@ -3167,12 +3165,14 @@ ifsettso(struct ifnet *ifp, int on)
> void
> ifa_add(struct ifnet *ifp, struct ifaddr *ifa)
> {
> + NET_ASSERT_LOCKED_EXCLUSIVE();
> TAILQ_INSERT_TAIL(>if_addrlist, ifa, ifa_list);
> }
>
> void
> ifa_del(struct ifnet *ifp, struct ifaddr *ifa)
> {
> + NET_ASSERT_LOCKED_EXCLUSIVE();
> TAILQ_REMOVE(>if_addrlist, ifa, ifa_list);
> }
>
> Index: sys/net/if_var.h
> ===
> RCS file: /cvs/src/sys/net/if_var.h,v
> retrieving revision 1.123
> diff -u -p -r1.123 if_var.h
> --- sys/net/if_var.h 5 Apr 2023 19:35:23 - 1.123
> +++ sys/net/if_var.h 18 Apr 2023 15:45:04 -
> @@ -240,7 +240,8 @@ struct ifaddr {
> #define ifa_broadaddr ifa_dstaddr /* broadcast address interface
> */
> struct sockaddr *ifa_netmask; /* used to determine subnet */
> struct ifnet *ifa_ifp; /* back-pointer to interface */
> - TAILQ_ENTRY(ifaddr) ifa_list; /* list of addresses for interface */
> + TAILQ_ENTRY(ifaddr) ifa_list; /* [N] list of addresses for
> + interface */
> u_int ifa_flags;
Re: Remove kernel lock from ifa_ifwithaddr()
On Tue, Apr 18, 2023 at 03:15:54PM +0200, Alexander Bluhm wrote:
> On Mon, Apr 17, 2023 at 03:16:36AM +0300, Vitaliy Makkoveev wrote:
> > Index: sys/dev/usb/if_umb.c
> This umb chunk is OK bluhm@
>
> > Index: sys/net/if.c
> > ===
> > RCS file: /cvs/src/sys/net/if.c,v
> > retrieving revision 1.688
> > diff -u -p -r1.688 if.c
> > --- sys/net/if.c8 Apr 2023 13:49:38 - 1.688
> > +++ sys/net/if.c17 Apr 2023 00:09:17 -
> > @@ -1409,8 +1409,9 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
>
> ifa_ifwithdstaddr() is iterating over the same loops. It should
> be changed together.
>
Done.
> if_unit() is iterating over if_list. There should be a NET_LOCK().
>
> TAILQ_ENTRY(ifnet) if_list; /* [K] all struct ifnets are chained
> */
> TAILQ_ENTRY(ifaddr) ifa_list; /* list of addresses for interface */
>
> Could you put an [N] there?
No. As I said in the "Call sysctl_ifnames()..." thread, we use both
kernel and net locks to protect `if_list'. This means we do
modification with both locks held, but for read only access only one
lock required. We need to do this because some places protected by
kernel lock do `if_list' foreach loops and we can't introduce context
switch. So, I proposed in the "Call sysctl_ifnames()..." thread mark
`if_list' as [NK] and add "XXXSMP:" comment above `ifnetlist'
definition. I posted the diff to that thread.
I have no objections to mark `ifa_list' as [N].
>
> Can we put an NET_ASSERT_LOCKED_EXCLUSIVE() in ifa_add() and ifa_del()?
>
Done.
> > Index: sys/net/rtsock.c
> > ===
> > RCS file: /cvs/src/sys/net/rtsock.c,v
> > retrieving revision 1.359
> > diff -u -p -r1.359 rtsock.c
> > --- sys/net/rtsock.c22 Jan 2023 12:05:44 - 1.359
> > +++ sys/net/rtsock.c17 Apr 2023 00:09:17 -
> > @@ -2374,18 +2374,18 @@ rt_setsource(unsigned int rtableid, stru
> > return (EAFNOSUPPORT);
> > }
> >
> > - KERNEL_LOCK();
> > + NET_LOCK();
> > /*
> > * Check if source address is assigned to an interface in the
> > * same rdomain
> > */
> > if ((ifa = ifa_ifwithaddr(src, rtableid)) == NULL) {
> > - KERNEL_UNLOCK();
> > + NET_UNLOCK();
> > return (EINVAL);
> > }
> >
> > error = rtable_setsource(rtableid, src->sa_family, ifa->ifa_addr);
> > - KERNEL_UNLOCK();
> > + NET_UNLOCK();
> >
> > return (error);
> > }
>
> I would perfer to put the NET_LOCK() into the caller. There are
> two more rtable_setsource() in this function which should be net
> locked.
>
rtable_setsource(... , NULL) actually doesn't destroy object pointed by
`ar_source', so for this call netlock is not required. However this
optimisation doesn't produce any visible effect, so no objections to
call rt_setsource() with netlock held.
ok?
Index: sys/dev/usb/if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.50
diff -u -p -r1.50 if_umb.c
--- sys/dev/usb/if_umb.c31 Mar 2023 23:53:49 - 1.50
+++ sys/dev/usb/if_umb.c18 Apr 2023 15:45:04 -
@@ -1829,6 +1829,7 @@ umb_add_inet_config(struct umb_softc *sc
default_sin.sin_len = sizeof (default_sin);
memset(, 0, sizeof(info));
+ NET_LOCK();
info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
info.rti_ifa = ifa_ifwithaddr(sintosa(_addr),
ifp->if_rdomain);
@@ -1836,7 +1837,6 @@ umb_add_inet_config(struct umb_softc *sc
info.rti_info[RTAX_NETMASK] = sintosa(_sin);
info.rti_info[RTAX_GATEWAY] = sintosa(_dstaddr);
- NET_LOCK();
rv = rtrequest(RTM_ADD, , 0, , ifp->if_rdomain);
NET_UNLOCK();
if (rv) {
@@ -1910,6 +1910,7 @@ umb_add_inet6_config(struct umb_softc *s
default_sin6.sin6_len = sizeof (default_sin6);
memset(, 0, sizeof(info));
+ NET_LOCK();
info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
info.rti_ifa = ifa_ifwithaddr(sin6tosa(_addr),
ifp->if_rdomain);
@@ -1917,7 +1918,6 @@ umb_add_inet6_config(struct umb_softc *s
info.rti_info[RTAX_NETMASK] = sin6tosa(_sin6);
info.rti_info[RTAX_GATEWAY] = sin6tosa(_dstaddr);
- NET_LOCK();
rv = rtrequest(RTM_ADD, , 0, , ifp->if_rdomain);
NET_UNLOCK();
if (rv) {
Index: sys/net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.688
diff -u -p -r1.688 if.c
--- sys/net/if.c8 Apr 2023 13:49:38 - 1.688
+++ sys/net/if.c18 Apr 2023 15:45:04 -
@@ -1409,8 +1409,9 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
struct ifaddr *ifa;
u_int rdomain;
+ NET_ASSERT_LOCKED();
+
rdomain = rtable_l2(rtableid);
- KERNEL_LOCK();
bgpd pass flowspec rules to RDE
This adds the needed bits to send the flowspec rules to the RDE.
The RDE just drops them on the ground for now.
--
:wq Claudio
Index: bgpd.c
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
retrieving revision 1.257
diff -u -p -r1.257 bgpd.c
--- bgpd.c 14 Feb 2023 15:33:46 - 1.257
+++ bgpd.c 18 Apr 2023 15:15:36 -
@@ -598,6 +598,7 @@ send_config(struct bgpd_config *conf)
struct roa *roa;
struct aspa_set *aspa;
struct rtr_config *rtr;
+ struct flowspec_config *f, *nf;
reconfpending = 3; /* one per child */
@@ -655,6 +656,26 @@ send_config(struct bgpd_config *conf)
/* networks go via kroute to the RDE */
kr_net_reload(conf->default_tableid, 0, >networks);
+
+ /* flowspec goes directly to the RDE, also remove old objects */
+ RB_FOREACH_SAFE(f, flowspec_tree, >flowspecs, nf) {
+ if (f->reconf_action != RECONF_DELETE) {
+ if (imsg_compose(ibuf_rde, IMSG_FLOWSPEC_ADD, 0, 0, -1,
+ f->flow, FLOWSPEC_SIZE + f->flow->len) == -1)
+ return (-1);
+ if (send_filterset(ibuf_rde, >attrset) == -1)
+ return (-1);
+ if (imsg_compose(ibuf_rde, IMSG_FLOWSPEC_DONE, 0, 0, -1,
+ NULL, 0) == -1)
+ return (-1);
+ } else {
+ if (imsg_compose(ibuf_rde, IMSG_FLOWSPEC_REMOVE, 0, 0,
+ -1, f->flow, FLOWSPEC_SIZE + f->flow->len) == -1)
+ return (-1);
+ RB_REMOVE(flowspec_tree, >flowspecs, f);
+ flowspec_free(f);
+ }
+ }
/* prefixsets for filters in the RDE */
while ((ps = SIMPLEQ_FIRST(>prefixsets)) != NULL) {
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.472
diff -u -p -r1.472 bgpd.h
--- bgpd.h 18 Apr 2023 12:11:27 - 1.472
+++ bgpd.h 18 Apr 2023 15:15:36 -
@@ -526,7 +526,7 @@ struct network {
struct flowspec {
uint16_tlen;
uint8_t aid;
- uint8_t pad;
+ uint8_t flags;
uint8_t data[1];
};
#define FLOWSPEC_SIZE (offsetof(struct flowspec, data))
@@ -613,6 +613,9 @@ enum imsg_type {
IMSG_NETWORK_REMOVE,
IMSG_NETWORK_FLUSH,
IMSG_NETWORK_DONE,
+ IMSG_FLOWSPEC_ADD,
+ IMSG_FLOWSPEC_DONE,
+ IMSG_FLOWSPEC_REMOVE,
IMSG_FILTER_SET,
IMSG_SOCKET_CONN,
IMSG_SOCKET_CONN_CTL,
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.601
diff -u -p -r1.601 rde.c
--- rde.c 13 Apr 2023 15:51:16 - 1.601
+++ rde.c 18 Apr 2023 15:18:19 -
@@ -102,6 +102,10 @@ voidnetwork_delete(struct network_con
static void network_dump_upcall(struct rib_entry *, void *);
static void network_flush_upcall(struct rib_entry *, void *);
+voidflowspec_add(struct flowspec *, struct filterstate *,
+ struct filter_set_head *);
+voidflowspec_delete(struct flowspec *);
+
voidrde_shutdown(void);
static int ovs_match(struct prefix *, uint32_t);
static int avs_match(struct prefix *, uint32_t);
@@ -719,6 +723,7 @@ rde_dispatch_imsg_parent(struct imsgbuf
static struct rde_prefixset *last_prefixset;
static struct as_set*last_as_set;
static struct l3vpn *vpn;
+ static struct flowspec *curflow;
struct imsg imsg;
struct mrt xmrt;
struct roa roa;
@@ -817,6 +822,75 @@ rde_dispatch_imsg_parent(struct imsgbuf
TAILQ_INIT(_p.attrset);
network_delete(_p);
break;
+ case IMSG_FLOWSPEC_ADD:
+ if (imsg.hdr.len - IMSG_HEADER_SIZE <= FLOWSPEC_SIZE) {
+ log_warnx("rde_dispatch: wrong imsg len");
+ break;
+ }
+ if (curflow != NULL) {
+ log_warnx("rde_dispatch: "
+ "unexpected flowspec add");
+ break;
+ }
+ curflow = malloc(imsg.hdr.len - IMSG_HEADER_SIZE);
+ if (curflow == NULL)
+ fatal(NULL);
+ memcpy(curflow, imsg.data,
+ imsg.hdr.len - IMSG_HEADER_SIZE);
+
Re: Call sysctl_source() with shared netlock
On Tue, Apr 18, 2023 at 03:36:09PM +0200, Alexander Bluhm wrote:
> On Mon, Apr 17, 2023 at 05:53:28PM +0200, Alexander Bluhm wrote:
> > On Mon, Apr 17, 2023 at 04:32:13PM +0200, Alexander Bluhm wrote:
> > > On Mon, Apr 17, 2023 at 02:36:57AM +0300, Vitaliy Makkoveev wrote:
> > > > It seems rt_setsource() needs some attention, but sysctl_source() could
> > > > be called with shared netlock just now.
> > >
> > > I think rtable_setsource() is not MP safe. It is documented as [K]
> > > kernel lock. But that is not true and makes no sense. It should
> > > be exclusive netlock. in_pcbselsrc() calls rtable_getsource() with
> > > netlock. We should rename source to ar_source so we can grep for
> > > its users.
> >
> > Perhaps something like this. I will run it through regress.
>
> Rebased after your rename commit. Regress test did not trigger
> lock assertions. ar_source is a pointer to an ifa_addr, so net
> lock should be safe.
>
> ok?
>
ok mvs@
> bluhm
>
> Index: net/art.h
> ===
> RCS file: /cvs/src/sys/net/art.h,v
> retrieving revision 1.22
> diff -u -p -r1.22 art.h
> --- net/art.h 18 Apr 2023 10:19:16 - 1.22
> +++ net/art.h 18 Apr 2023 13:32:46 -
> @@ -41,7 +41,7 @@ struct art_root {
> uint8_t ar_nlvl; /* [I] Number of levels */
> uint8_t ar_alen; /* [I] Address length in bits */
> uint8_t ar_off;/* [I] Offset of key in bytes */
> - struct sockaddr *ar_source; /* [K] optional src addr to use
> */
> + struct sockaddr *ar_source; /* [N] use optional src addr */
> };
>
> #define ISLEAF(e)(((unsigned long)(e) & 1) == 0)
> Index: net/rtable.c
> ===
> RCS file: /cvs/src/sys/net/rtable.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 rtable.c
> --- net/rtable.c 18 Apr 2023 10:19:16 - 1.81
> +++ net/rtable.c 18 Apr 2023 13:32:46 -
> @@ -376,6 +376,8 @@ rtable_setsource(unsigned int rtableid,
> {
> struct art_root *ar;
>
> + NET_ASSERT_LOCKED_EXCLUSIVE();
> +
> if ((ar = rtable_get(rtableid, af)) == NULL)
> return (EAFNOSUPPORT);
>
> @@ -388,6 +390,8 @@ struct sockaddr *
> rtable_getsource(unsigned int rtableid, int af)
> {
> struct art_root *ar;
> +
> + NET_ASSERT_LOCKED();
>
> ar = rtable_get(rtableid, af);
> if (ar == NULL)
> Index: net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.362
> diff -u -p -r1.362 rtsock.c
> --- net/rtsock.c 18 Apr 2023 09:56:54 - 1.362
> +++ net/rtsock.c 18 Apr 2023 13:32:46 -
> @@ -853,8 +853,10 @@ route_output(struct mbuf *m, struct sock
> error = EINVAL;
> goto fail;
> }
> - if ((error =
> - rt_setsource(tableid, info.rti_info[RTAX_IFA])) != 0)
> + NET_LOCK();
> + error = rt_setsource(tableid, info.rti_info[RTAX_IFA]);
> + NET_UNLOCK();
> + if (error)
> goto fail;
> } else {
> error = rtm_output(rtm, , , prio, tableid);
> @@ -862,7 +864,9 @@ route_output(struct mbuf *m, struct sock
> type = rtm->rtm_type;
> seq = rtm->rtm_seq;
> free(rtm, M_RTABLE, len);
> + NET_LOCK_SHARED();
> rtm = rtm_report(rt, type, seq, tableid);
> + NET_UNLOCK_SHARED();
> len = rtm->rtm_msglen;
> }
> }
>
Re: bgpd print flowspec definitions in printconf
On Tue, Apr 18, 2023 at 03:11:29PM +0200, Claudio Jeker wrote: > This adds the bit to show flowspec rules in the printconfig output > when run with bgpd -nvf config. > > I did not fix the ICMP handling yet. It feels like too much of an edge > case for now. Reads fine ok tb
bgpd add flowspec support to rde_prefix.c
Extend the pt_entry api to handle flowspec.
Introduce pt_get_flow() and pt_add_flow() to lookup and insert flowspec
objects. Add pt_getflowspec() which works somewhat similar to pt_getaddr()
to extract the flowspec NLRI from a pt_entry.
There is a hack in pt_getaddr() to return something. This is the
destination prefix of the flowspec rule (or a default route if that is
missing). Also extend pt_write() to handle flowspec NLRI.
--
:wq Claudio
Index: rde.h
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
retrieving revision 1.291
diff -u -p -r1.291 rde.h
--- rde.h 7 Apr 2023 13:49:03 - 1.291
+++ rde.h 18 Apr 2023 13:18:48 -
@@ -512,9 +512,12 @@ enum filter_actions rde_filter(struct fi
voidpt_init(void);
voidpt_shutdown(void);
voidpt_getaddr(struct pt_entry *, struct bgpd_addr *);
+int pt_getflowspec(struct pt_entry *, uint8_t **);
struct pt_entry*pt_fill(struct bgpd_addr *, int);
struct pt_entry*pt_get(struct bgpd_addr *, int);
struct pt_entry *pt_add(struct bgpd_addr *, int);
+struct pt_entry*pt_get_flow(struct flowspec *);
+struct pt_entry*pt_add_flow(struct flowspec *);
voidpt_remove(struct pt_entry *);
struct pt_entry*pt_lookup(struct bgpd_addr *);
int pt_prefix_cmp(const struct pt_entry *, const struct pt_entry *);
Index: rde_prefix.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v
retrieving revision 1.48
diff -u -p -r1.48 rde_prefix.c
--- rde_prefix.c30 Mar 2023 13:25:23 - 1.48
+++ rde_prefix.c18 Apr 2023 13:18:48 -
@@ -95,6 +95,17 @@ struct pt_entry_vpn6 {
uint8_t pad2;
};
+struct pt_entry_flow {
+ RB_ENTRY(pt_entry) pt_e;
+ uint8_t aid;
+ uint8_t prefixlen; /* unused ??? */
+ uint16_tlen;
+ uint32_trefcnt;
+ uint64_trd;
+ uint8_t flow[1];/* NLRI */
+};
+
+#define PT_FLOW_SIZE (offsetof(struct pt_entry_flow, flow))
RB_HEAD(pt_tree, pt_entry);
RB_PROTOTYPE(pt_tree, pt_entry, pt_e, pt_prefix_cmp);
@@ -118,6 +129,8 @@ pt_shutdown(void)
void
pt_getaddr(struct pt_entry *pte, struct bgpd_addr *addr)
{
+ struct pt_entry_flow*pflow;
+
memset(addr, 0, sizeof(struct bgpd_addr));
addr->aid = pte->aid;
switch (addr->aid) {
@@ -144,11 +157,34 @@ pt_getaddr(struct pt_entry *pte, struct
((struct pt_entry_vpn6 *)pte)->labelstack,
addr->labellen);
break;
+ case AID_FLOWSPECv4:
+ case AID_FLOWSPECv6:
+ pflow = (struct pt_entry_flow *)pte;
+ flowspec_get_addr(pflow->flow, pflow->len - PT_FLOW_SIZE,
+ FLOWSPEC_TYPE_DEST, addr->aid == AID_FLOWSPECv6,
+ addr, >prefixlen, NULL);
+ break;
default:
fatalx("pt_getaddr: unknown af");
}
}
+int
+pt_getflowspec(struct pt_entry *pte, uint8_t **flow)
+{
+ struct pt_entry_flow*pflow;
+
+ switch (pte->aid) {
+ case AID_FLOWSPECv4:
+ case AID_FLOWSPECv6:
+ pflow = (struct pt_entry_flow *)pte;
+ *flow = pflow->flow;
+ return pflow->len - PT_FLOW_SIZE;
+ default:
+ fatalx("pt_getflowspec: unknown af");
+ }
+}
+
struct pt_entry *
pt_fill(struct bgpd_addr *prefix, int prefixlen)
{
@@ -234,6 +270,48 @@ pt_add(struct bgpd_addr *prefix, int pre
return (p);
}
+struct pt_entry *
+pt_get_flow(struct flowspec *f)
+{
+ struct pt_entry *needle;
+ union {
+ struct pt_entry_flowflow;
+ uint8_t buf[4096];
+ } x;
+
+ needle = (struct pt_entry *)
+
+ memset(needle, 0, PT_FLOW_SIZE);
+ needle->aid = f->aid;
+ needle->len = f->len + PT_FLOW_SIZE;
+ memcpy(((struct pt_entry_flow *)needle)->flow, f->data, f->len);
+
+ return RB_FIND(pt_tree, , (struct pt_entry *)needle);
+}
+
+struct pt_entry *
+pt_add_flow(struct flowspec *f)
+{
+ struct pt_entry *p;
+ int len = f->len + PT_FLOW_SIZE;
+
+ p = malloc(len);
+ if (p == NULL)
+ fatal(__func__);
+ rdemem.pt_cnt[f->aid]++;
+ rdemem.pt_size[f->aid] += len;
+ memset(p, 0, PT_FLOW_SIZE);
+
+ p->len = len;
+ p->aid = f->aid;
+ memcpy(((struct pt_entry_flow *)p)->flow, f->data, f->len);
+
+ if (RB_INSERT(pt_tree, , p) != NULL)
+ fatalx("pt_add: insert failed");
+
+ return (p);
+}
+
void
pt_remove(struct pt_entry *pte)
{
@@ -278,6 +356,7 @@ pt_prefix_cmp(const struct pt_entry *a,
const struct pt_entry6 *a6, *b6;
Re: Call sysctl_source() with shared netlock
On Mon, Apr 17, 2023 at 05:53:28PM +0200, Alexander Bluhm wrote:
> On Mon, Apr 17, 2023 at 04:32:13PM +0200, Alexander Bluhm wrote:
> > On Mon, Apr 17, 2023 at 02:36:57AM +0300, Vitaliy Makkoveev wrote:
> > > It seems rt_setsource() needs some attention, but sysctl_source() could
> > > be called with shared netlock just now.
> >
> > I think rtable_setsource() is not MP safe. It is documented as [K]
> > kernel lock. But that is not true and makes no sense. It should
> > be exclusive netlock. in_pcbselsrc() calls rtable_getsource() with
> > netlock. We should rename source to ar_source so we can grep for
> > its users.
>
> Perhaps something like this. I will run it through regress.
Rebased after your rename commit. Regress test did not trigger
lock assertions. ar_source is a pointer to an ifa_addr, so net
lock should be safe.
ok?
bluhm
Index: net/art.h
===
RCS file: /cvs/src/sys/net/art.h,v
retrieving revision 1.22
diff -u -p -r1.22 art.h
--- net/art.h 18 Apr 2023 10:19:16 - 1.22
+++ net/art.h 18 Apr 2023 13:32:46 -
@@ -41,7 +41,7 @@ struct art_root {
uint8_t ar_nlvl; /* [I] Number of levels */
uint8_t ar_alen; /* [I] Address length in bits */
uint8_t ar_off;/* [I] Offset of key in bytes */
- struct sockaddr *ar_source; /* [K] optional src addr to use
*/
+ struct sockaddr *ar_source; /* [N] use optional src addr */
};
#define ISLEAF(e) (((unsigned long)(e) & 1) == 0)
Index: net/rtable.c
===
RCS file: /cvs/src/sys/net/rtable.c,v
retrieving revision 1.81
diff -u -p -r1.81 rtable.c
--- net/rtable.c18 Apr 2023 10:19:16 - 1.81
+++ net/rtable.c18 Apr 2023 13:32:46 -
@@ -376,6 +376,8 @@ rtable_setsource(unsigned int rtableid,
{
struct art_root *ar;
+ NET_ASSERT_LOCKED_EXCLUSIVE();
+
if ((ar = rtable_get(rtableid, af)) == NULL)
return (EAFNOSUPPORT);
@@ -388,6 +390,8 @@ struct sockaddr *
rtable_getsource(unsigned int rtableid, int af)
{
struct art_root *ar;
+
+ NET_ASSERT_LOCKED();
ar = rtable_get(rtableid, af);
if (ar == NULL)
Index: net/rtsock.c
===
RCS file: /cvs/src/sys/net/rtsock.c,v
retrieving revision 1.362
diff -u -p -r1.362 rtsock.c
--- net/rtsock.c18 Apr 2023 09:56:54 - 1.362
+++ net/rtsock.c18 Apr 2023 13:32:46 -
@@ -853,8 +853,10 @@ route_output(struct mbuf *m, struct sock
error = EINVAL;
goto fail;
}
- if ((error =
- rt_setsource(tableid, info.rti_info[RTAX_IFA])) != 0)
+ NET_LOCK();
+ error = rt_setsource(tableid, info.rti_info[RTAX_IFA]);
+ NET_UNLOCK();
+ if (error)
goto fail;
} else {
error = rtm_output(rtm, , , prio, tableid);
@@ -862,7 +864,9 @@ route_output(struct mbuf *m, struct sock
type = rtm->rtm_type;
seq = rtm->rtm_seq;
free(rtm, M_RTABLE, len);
+ NET_LOCK_SHARED();
rtm = rtm_report(rt, type, seq, tableid);
+ NET_UNLOCK_SHARED();
len = rtm->rtm_msglen;
}
}
Re: pfctl + bgpd for loop ugliness
Le Tue, Apr 18, 2023 at 02:43:26PM +0200, Theo Buehler a écrit :
> On Tue, Apr 18, 2023 at 02:06:46PM +0200, Claudio Jeker wrote:
> > This and the others are IIRC streight from pfctl. So if someone wants a
> > free commit :)
>
> How about this. pfctl and bgpd are the same, except that the bgpd one
> has a bsearch() nitems on top. pfctl regress is happy.
>
Looks good to me. OK denis@
> Index: sbin/pfctl/pfctl_parser.c
> ===
> RCS file: /cvs/src/sbin/pfctl/pfctl_parser.c,v
> retrieving revision 1.347
> diff -u -p -r1.347 pfctl_parser.c
> --- sbin/pfctl/pfctl_parser.c 9 Nov 2022 23:00:00 - 1.347
> +++ sbin/pfctl/pfctl_parser.c 18 Apr 2023 12:37:19 -
> @@ -62,6 +62,10 @@
> #include "pfctl_parser.h"
> #include "pfctl.h"
>
> +#ifndef nitems
> +#define nitems(_a) (sizeof((_a)) / sizeof((_a)[0]))
> +#endif
> +
> void print_op (u_int8_t, const char *, const char *);
> void print_port (u_int8_t, u_int16_t, u_int16_t, const char *, int);
> void print_ugid (u_int8_t, id_t, id_t, const char *);
> @@ -224,17 +228,15 @@ copy_satopfaddr(struct pf_addr *pfa, str
> const struct icmptypeent *
> geticmptypebynumber(u_int8_t type, sa_family_t af)
> {
> - unsigned inti;
> + size_t i;
>
> if (af != AF_INET6) {
> - for (i=0; i < (sizeof (icmp_type) / sizeof(icmp_type[0]));
> - i++) {
> + for (i = 0; i < nitems(icmp_type); i++) {
> if (type == icmp_type[i].type)
> return (_type[i]);
> }
> } else {
> - for (i=0; i < (sizeof (icmp6_type) /
> - sizeof(icmp6_type[0])); i++) {
> + for (i = 0; i < nitems(icmp6_type); i++) {
> if (type == icmp6_type[i].type)
>return (_type[i]);
> }
> @@ -245,17 +247,15 @@ geticmptypebynumber(u_int8_t type, sa_fa
> const struct icmptypeent *
> geticmptypebyname(char *w, sa_family_t af)
> {
> - unsigned inti;
> + size_t i;
>
> if (af != AF_INET6) {
> - for (i=0; i < (sizeof (icmp_type) / sizeof(icmp_type[0]));
> - i++) {
> + for (i = 0; i < nitems(icmp_type); i++) {
> if (!strcmp(w, icmp_type[i].name))
> return (_type[i]);
> }
> } else {
> - for (i=0; i < (sizeof (icmp6_type) /
> - sizeof(icmp6_type[0])); i++) {
> + for (i = 0; i < nitems(icmp6_type); i++) {
> if (!strcmp(w, icmp6_type[i].name))
> return (_type[i]);
> }
> @@ -266,18 +266,16 @@ geticmptypebyname(char *w, sa_family_t a
> const struct icmpcodeent *
> geticmpcodebynumber(u_int8_t type, u_int8_t code, sa_family_t af)
> {
> - unsigned inti;
> + size_t i;
>
> if (af != AF_INET6) {
> - for (i=0; i < (sizeof (icmp_code) / sizeof(icmp_code[0]));
> - i++) {
> + for (i = 0; i < nitems(icmp_code); i++) {
> if (type == icmp_code[i].type &&
> code == icmp_code[i].code)
> return (_code[i]);
> }
> } else {
> - for (i=0; i < (sizeof (icmp6_code) /
> - sizeof(icmp6_code[0])); i++) {
> + for (i = 0; i < nitems(icmp6_code); i++) {
> if (type == icmp6_code[i].type &&
> code == icmp6_code[i].code)
> return (_code[i]);
> @@ -289,18 +287,16 @@ geticmpcodebynumber(u_int8_t type, u_int
> const struct icmpcodeent *
> geticmpcodebyname(u_long type, char *w, sa_family_t af)
> {
> - unsigned inti;
> + size_t i;
>
> if (af != AF_INET6) {
> - for (i=0; i < (sizeof (icmp_code) / sizeof(icmp_code[0]));
> - i++) {
> + for (i = 0; i < nitems(icmp_code); i++) {
> if (type == icmp_code[i].type &&
> !strcmp(w, icmp_code[i].name))
> return (_code[i]);
> }
> } else {
> - for (i=0; i < (sizeof (icmp6_code) /
> - sizeof(icmp6_code[0])); i++) {
> + for (i = 0; i < nitems(icmp6_code); i++) {
> if (type == icmp6_code[i].type &&
> !strcmp(w, icmp6_code[i].name))
> return (_code[i]);
> Index: usr.sbin/bgpd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.447
> diff -u -p -r1.447 parse.y
> --- usr.sbin/bgpd/parse.y 18 Apr 2023 12:11:27 - 1.447
> +++ usr.sbin/bgpd/parse.y 18 Apr 2023 12:37:19 -
> @@ -52,6 +52,10 @@
> #include
Re: Remove kernel lock from ifa_ifwithaddr()
On Mon, Apr 17, 2023 at 03:16:36AM +0300, Vitaliy Makkoveev wrote:
> Index: sys/dev/usb/if_umb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.50
> diff -u -p -r1.50 if_umb.c
> --- sys/dev/usb/if_umb.c 31 Mar 2023 23:53:49 - 1.50
> +++ sys/dev/usb/if_umb.c 17 Apr 2023 00:09:17 -
> @@ -1829,6 +1829,7 @@ umb_add_inet_config(struct umb_softc *sc
> default_sin.sin_len = sizeof (default_sin);
>
> memset(, 0, sizeof(info));
> + NET_LOCK();
> info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> info.rti_ifa = ifa_ifwithaddr(sintosa(_addr),
> ifp->if_rdomain);
> @@ -1836,7 +1837,6 @@ umb_add_inet_config(struct umb_softc *sc
> info.rti_info[RTAX_NETMASK] = sintosa(_sin);
> info.rti_info[RTAX_GATEWAY] = sintosa(_dstaddr);
>
> - NET_LOCK();
> rv = rtrequest(RTM_ADD, , 0, , ifp->if_rdomain);
> NET_UNLOCK();
> if (rv) {
> @@ -1910,6 +1910,7 @@ umb_add_inet6_config(struct umb_softc *s
> default_sin6.sin6_len = sizeof (default_sin6);
>
> memset(, 0, sizeof(info));
> + NET_LOCK();
> info.rti_flags = RTF_GATEWAY /* maybe | RTF_STATIC */;
> info.rti_ifa = ifa_ifwithaddr(sin6tosa(_addr),
> ifp->if_rdomain);
> @@ -1917,7 +1918,6 @@ umb_add_inet6_config(struct umb_softc *s
> info.rti_info[RTAX_NETMASK] = sin6tosa(_sin6);
> info.rti_info[RTAX_GATEWAY] = sin6tosa(_dstaddr);
>
> - NET_LOCK();
> rv = rtrequest(RTM_ADD, , 0, , ifp->if_rdomain);
> NET_UNLOCK();
> if (rv) {
This umb chunk is OK bluhm@
> Index: sys/net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.688
> diff -u -p -r1.688 if.c
> --- sys/net/if.c 8 Apr 2023 13:49:38 - 1.688
> +++ sys/net/if.c 17 Apr 2023 00:09:17 -
> @@ -1409,8 +1409,9 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
> struct ifaddr *ifa;
> u_int rdomain;
>
> + NET_ASSERT_LOCKED();
> +
> rdomain = rtable_l2(rtableid);
> - KERNEL_LOCK();
> TAILQ_FOREACH(ifp, , if_list) {
> if (ifp->if_rdomain != rdomain)
> continue;
> @@ -1420,12 +1421,10 @@ ifa_ifwithaddr(struct sockaddr *addr, u_
> continue;
>
> if (equal(addr, ifa->ifa_addr)) {
> - KERNEL_UNLOCK();
> return (ifa);
> }
> }
> }
> - KERNEL_UNLOCK();
> return (NULL);
> }
ifa_ifwithdstaddr() is iterating over the same loops. It should
be changed together.
if_unit() is iterating over if_list. There should be a NET_LOCK().
TAILQ_ENTRY(ifnet) if_list; /* [K] all struct ifnets are chained */
TAILQ_ENTRY(ifaddr) ifa_list; /* list of addresses for interface */
Could you put an [N] there?
Can we put an NET_ASSERT_LOCKED_EXCLUSIVE() in ifa_add() and ifa_del()?
> Index: sys/net/rtsock.c
> ===
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.359
> diff -u -p -r1.359 rtsock.c
> --- sys/net/rtsock.c 22 Jan 2023 12:05:44 - 1.359
> +++ sys/net/rtsock.c 17 Apr 2023 00:09:17 -
> @@ -2374,18 +2374,18 @@ rt_setsource(unsigned int rtableid, stru
> return (EAFNOSUPPORT);
> }
>
> - KERNEL_LOCK();
> + NET_LOCK();
> /*
>* Check if source address is assigned to an interface in the
>* same rdomain
>*/
> if ((ifa = ifa_ifwithaddr(src, rtableid)) == NULL) {
> - KERNEL_UNLOCK();
> + NET_UNLOCK();
> return (EINVAL);
> }
>
> error = rtable_setsource(rtableid, src->sa_family, ifa->ifa_addr);
> - KERNEL_UNLOCK();
> + NET_UNLOCK();
>
> return (error);
> }
I would perfer to put the NET_LOCK() into the caller. There are
two more rtable_setsource() in this function which should be net
locked.
bgpd print flowspec definitions in printconf
This adds the bit to show flowspec rules in the printconfig output
when run with bgpd -nvf config.
I did not fix the ICMP handling yet. It feels like too much of an edge
case for now.
--
:wq Claudio
Index: printconf.c
===
RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
retrieving revision 1.164
diff -u -p -r1.164 printconf.c
--- printconf.c 9 Mar 2023 13:12:19 - 1.164
+++ printconf.c 18 Apr 2023 13:00:37 -
@@ -38,6 +38,7 @@ void print_l3vpn_targets(struct filter
voidprint_l3vpn(struct l3vpn *);
const char *print_af(uint8_t);
voidprint_network(struct network_config *, const char *);
+voidprint_flowspec(struct flowspec_config *, const char *);
voidprint_as_sets(struct as_set_head *);
voidprint_prefixsets(struct prefixset_head *);
voidprint_originsets(struct prefixset_head *);
@@ -461,12 +462,12 @@ print_af(uint8_t aid)
{
/*
* Hack around the fact that aid2str() will return "IPv4 unicast"
-* for AID_INET. AID_INET and AID_INET6 need special handling and
-* the other AID should never end up here (at least for now).
+* for AID_INET. AID_INET, AID_INET6 and the flowspec AID need
+* special handling and the other AID should never end up here.
*/
- if (aid == AID_INET)
+ if (aid == AID_INET || aid == AID_FLOWSPECv4)
return ("inet");
- if (aid == AID_INET6)
+ if (aid == AID_INET6 || aid == AID_FLOWSPECv6)
return ("inet6");
return (aid2str(aid));
}
@@ -503,6 +504,112 @@ print_network(struct network_config *n,
printf("\n");
}
+static void
+print_flowspec_list(struct flowspec *f, int type, int is_v6)
+{
+ const uint8_t *comp;
+ const char *fmt;
+ int complen, off = 0;
+
+ if (flowspec_get_component(f->data, f->len, type, is_v6,
+ , ) != 1)
+ return;
+
+ printf("%s ", flowspec_fmt_label(type));
+ fmt = flowspec_fmt_num_op(comp, complen, );
+ if (off == -1) {
+ printf("%s ", fmt);
+ } else {
+ printf("{ %s ", fmt);
+ do {
+ fmt = flowspec_fmt_num_op(comp, complen, );
+ printf("%s ", fmt);
+ } while (off != -1);
+ printf("} ");
+ }
+}
+
+static void
+print_flowspec_flags(struct flowspec *f, int type, int is_v6)
+{
+ const uint8_t *comp;
+ const char *fmt, *flags;
+ int complen, off = 0;
+
+ if (flowspec_get_component(f->data, f->len, type, is_v6,
+ , ) != 1)
+ return;
+
+ printf("%s ", flowspec_fmt_label(type));
+
+ switch (type) {
+ case FLOWSPEC_TYPE_TCP_FLAGS:
+ flags = FLOWSPEC_TCP_FLAG_STRING;
+ break;
+ case FLOWSPEC_TYPE_FRAG:
+ if (!is_v6)
+ flags = FLOWSPEC_FRAG_STRING4;
+ else
+ flags = FLOWSPEC_FRAG_STRING6;
+ break;
+ }
+
+ fmt = flowspec_fmt_bin_op(comp, complen, , flags);
+ if (off == -1) {
+ printf("%s ", fmt);
+ } else {
+ printf("{ %s ", fmt);
+ do {
+ fmt = flowspec_fmt_bin_op(comp, complen, , flags);
+ printf("%s ", fmt);
+ } while (off != -1);
+ printf("} ");
+ }
+}
+
+static void
+print_flowspec_addr(struct flowspec *f, int type, int is_v6)
+{
+ struct bgpd_addr addr;
+ uint8_t plen;
+
+ flowspec_get_addr(f->data, f->len, type, is_v6, , , NULL);
+ if (plen == 0)
+ printf("%s any ", flowspec_fmt_label(type));
+ else
+ printf("%s %s/%u ", flowspec_fmt_label(type),
+ log_addr(), plen);
+}
+
+void
+print_flowspec(struct flowspec_config *fconf, const char *c)
+{
+ struct flowspec *f = fconf->flow;
+ int is_v6 = (f->aid == AID_FLOWSPECv6);
+
+ printf("%sflowspec %s ", c, print_af(f->aid));
+
+ print_flowspec_list(f, FLOWSPEC_TYPE_PROTO, is_v6);
+
+ print_flowspec_addr(f, FLOWSPEC_TYPE_SOURCE, is_v6);
+ print_flowspec_list(f, FLOWSPEC_TYPE_SRC_PORT, is_v6);
+
+ print_flowspec_addr(f, FLOWSPEC_TYPE_DEST, is_v6);
+ print_flowspec_list(f, FLOWSPEC_TYPE_DST_PORT, is_v6);
+
+ print_flowspec_list(f, FLOWSPEC_TYPE_DSCP, is_v6);
+ print_flowspec_list(f, FLOWSPEC_TYPE_PKT_LEN, is_v6);
+ print_flowspec_flags(f, FLOWSPEC_TYPE_TCP_FLAGS, is_v6);
+ print_flowspec_flags(f, FLOWSPEC_TYPE_FRAG, is_v6);
+
+ /* TODO: fixup the code handling to be like in the parser */
+ print_flowspec_list(f, FLOWSPEC_TYPE_ICMP_TYPE, is_v6);
+ print_flowspec_list(f, FLOWSPEC_TYPE_ICMP_CODE, is_v6);
+
+ print_set(>attrset);
+ printf("\n");
+}
+
void
Re: xhci(4): map MSI-X
I can't recall a real device that worked with MSI but not with msi-x, so
I'm ok with chucking it in.
On Tue, 18 Apr 2023, 21:15 Patrick Wildt, wrote:
> Hi,
>
> I noticed that on Qemu with arm64 we're falling back to legacy
> interrupts with xhci(4) on PCI. Turns out that the virtual xHCI
> does not support MSI, but it does support MSI-X. By having xhci(4)
> map MSI-X as well I can use that on Qemu.
>
> My x395, which so far used MSI, is still working fine with this:
>
> -xhci0 at pci6 dev 0 function 3 "AMD 17h/1xh xHCI" rev 0x00: msi, xHCI 1.10
> +xhci0 at pci6 dev 0 function 3 "AMD 17h/1xh xHCI" rev 0x00: msix, xHCI
> 1.10
> -xhci1 at pci6 dev 0 function 4 "AMD 17h/1xh xHCI" rev 0x00: msi, xHCI 1.10
> +xhci1 at pci6 dev 0 function 4 "AMD 17h/1xh xHCI" rev 0x00: msix, xHCI
> 1.10
>
> ok?
>
> Patrick
>
> diff --git a/sys/dev/pci/xhci_pci.c b/sys/dev/pci/xhci_pci.c
> index 88bdf66dbd74..52d0bc1b6761 100644
> --- a/sys/dev/pci/xhci_pci.c
> +++ b/sys/dev/pci/xhci_pci.c
> @@ -158,7 +158,8 @@ xhci_pci_attach(struct device *parent, struct device
> *self, void *aux)
> }
>
> /* Map and establish the interrupt. */
> - if (pci_intr_map_msi(pa, ) != 0 && pci_intr_map(pa, ) != 0) {
> + if (pci_intr_map_msix(pa, 0, ) != 0 &&
> + pci_intr_map_msi(pa, ) != 0 && pci_intr_map(pa, ) != 0) {
> printf(": couldn't map interrupt\n");
> goto unmap_ret;
> }
>
>
pfctl + bgpd for loop ugliness
On Tue, Apr 18, 2023 at 02:06:46PM +0200, Claudio Jeker wrote:
> This and the others are IIRC streight from pfctl. So if someone wants a
> free commit :)
How about this. pfctl and bgpd are the same, except that the bgpd one
has a bsearch() nitems on top. pfctl regress is happy.
Index: sbin/pfctl/pfctl_parser.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl_parser.c,v
retrieving revision 1.347
diff -u -p -r1.347 pfctl_parser.c
--- sbin/pfctl/pfctl_parser.c 9 Nov 2022 23:00:00 - 1.347
+++ sbin/pfctl/pfctl_parser.c 18 Apr 2023 12:37:19 -
@@ -62,6 +62,10 @@
#include "pfctl_parser.h"
#include "pfctl.h"
+#ifndef nitems
+#define nitems(_a) (sizeof((_a)) / sizeof((_a)[0]))
+#endif
+
voidprint_op (u_int8_t, const char *, const char *);
voidprint_port (u_int8_t, u_int16_t, u_int16_t, const char *, int);
voidprint_ugid (u_int8_t, id_t, id_t, const char *);
@@ -224,17 +228,15 @@ copy_satopfaddr(struct pf_addr *pfa, str
const struct icmptypeent *
geticmptypebynumber(u_int8_t type, sa_family_t af)
{
- unsigned inti;
+ size_t i;
if (af != AF_INET6) {
- for (i=0; i < (sizeof (icmp_type) / sizeof(icmp_type[0]));
- i++) {
+ for (i = 0; i < nitems(icmp_type); i++) {
if (type == icmp_type[i].type)
return (_type[i]);
}
} else {
- for (i=0; i < (sizeof (icmp6_type) /
- sizeof(icmp6_type[0])); i++) {
+ for (i = 0; i < nitems(icmp6_type); i++) {
if (type == icmp6_type[i].type)
return (_type[i]);
}
@@ -245,17 +247,15 @@ geticmptypebynumber(u_int8_t type, sa_fa
const struct icmptypeent *
geticmptypebyname(char *w, sa_family_t af)
{
- unsigned inti;
+ size_t i;
if (af != AF_INET6) {
- for (i=0; i < (sizeof (icmp_type) / sizeof(icmp_type[0]));
- i++) {
+ for (i = 0; i < nitems(icmp_type); i++) {
if (!strcmp(w, icmp_type[i].name))
return (_type[i]);
}
} else {
- for (i=0; i < (sizeof (icmp6_type) /
- sizeof(icmp6_type[0])); i++) {
+ for (i = 0; i < nitems(icmp6_type); i++) {
if (!strcmp(w, icmp6_type[i].name))
return (_type[i]);
}
@@ -266,18 +266,16 @@ geticmptypebyname(char *w, sa_family_t a
const struct icmpcodeent *
geticmpcodebynumber(u_int8_t type, u_int8_t code, sa_family_t af)
{
- unsigned inti;
+ size_t i;
if (af != AF_INET6) {
- for (i=0; i < (sizeof (icmp_code) / sizeof(icmp_code[0]));
- i++) {
+ for (i = 0; i < nitems(icmp_code); i++) {
if (type == icmp_code[i].type &&
code == icmp_code[i].code)
return (_code[i]);
}
} else {
- for (i=0; i < (sizeof (icmp6_code) /
- sizeof(icmp6_code[0])); i++) {
+ for (i = 0; i < nitems(icmp6_code); i++) {
if (type == icmp6_code[i].type &&
code == icmp6_code[i].code)
return (_code[i]);
@@ -289,18 +287,16 @@ geticmpcodebynumber(u_int8_t type, u_int
const struct icmpcodeent *
geticmpcodebyname(u_long type, char *w, sa_family_t af)
{
- unsigned inti;
+ size_t i;
if (af != AF_INET6) {
- for (i=0; i < (sizeof (icmp_code) / sizeof(icmp_code[0]));
- i++) {
+ for (i = 0; i < nitems(icmp_code); i++) {
if (type == icmp_code[i].type &&
!strcmp(w, icmp_code[i].name))
return (_code[i]);
}
} else {
- for (i=0; i < (sizeof (icmp6_code) /
- sizeof(icmp6_code[0])); i++) {
+ for (i = 0; i < nitems(icmp6_code); i++) {
if (type == icmp6_code[i].type &&
!strcmp(w, icmp6_code[i].name))
return (_code[i]);
Index: usr.sbin/bgpd/parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.447
diff -u -p -r1.447 parse.y
--- usr.sbin/bgpd/parse.y 18 Apr 2023 12:11:27 - 1.447
+++ usr.sbin/bgpd/parse.y 18 Apr 2023 12:37:19 -
@@ -52,6 +52,10 @@
#include "rde.h"
#include "log.h"
+#ifndef nitems
+#define nitems(_a) (sizeof((_a)) / sizeof((_a)[0]))
+#endif
+
#define MACRO_NAME_LEN 128
TAILQ_HEAD(files, file)
Re: bgpd flowspec parser
On Tue, Apr 18, 2023 at 12:52:00PM +0200, Theo Buehler wrote:
> On Tue, Apr 18, 2023 at 11:29:26AM +0200, Claudio Jeker wrote:
> > This diff adds the parse.y and config.c bits for flowspec.
> > I tried to make flowspec rules as similar to pf rules (even though
> > flowspec is more flexible).
> >
> > Now this diff does nothing in itself but is already large enough to not
> > add more to it. In parse.y the individual flowspec components are built up
> > in a flowspec context and then at the end converted into a struct flowspec
> > object. I wrapped that into a struct flowspec_config so that all the
> > parent config bits can be kept together. (For struct network this is
> > currently the other way around but I plan to change this at a later
> > point).
>
> ok tb
>
> Couple of nits and comments inline. Apart from two copy-paste errors
> (s/4/6) nothing important
>
> > +flowspec_alloc(uint8_t aid, int len)
> > +{
> > + struct flowspec_config *conf;
> > + struct flowspec *flow;
> > +
> > + flow = malloc(FLOWSPEC_SIZE + len);
> > + if (flow == NULL)
> > + return NULL;
> > + memset(flow, 0, FLOWSPEC_SIZE);
>
> I assume not zeroing len bytes is worth this extra dance as opposed
> to using calloc().
I went back and for between the two versions. I want to use zero the start
of struct flowspec because of the pad but it feels wasteful to zero
everything and then set all but 1 byte.
> > + /*
> > +* merge the flowspec statements, but first mark the old ones
> > +* for deletion. Which happens when the flowspec is sent to the RDE.
> > +*/
>
> This comment is a bit awkward. Maybe split the sentence differently like this:
>
> /*
>* Merge the flowspec statements. Mark the old ones for deletion
>* which happens when the flowspec is sent to the RDE.
>*/
Thanks, I used your version.
> > +parse_flags(char *s)
> > +{
> > + const char *flags = FLOWSPEC_TCP_FLAG_STRING;
> > + char *p, *q;
> > + uint8_t f = 0;
> > +
> > + if (curflow->type == FLOWSPEC_TYPE_FRAG) {
> > + if (curflow->aid == AID_INET)
> > + flags = FLOWSPEC_FRAG_STRING4;
> > + else
> > + flags = FLOWSPEC_FRAG_STRING4;
>
> s/4/6
>
> flags = FLOWSPEC_FRAG_STRING6;
Thanks for finding this and the other 6 vs 4 copy paste errors
> > +static int
> > +geticmptypebyname(char *w, uint8_t aid)
> > +{
> > + unsigned inti;
> > +
> > + switch (aid) {
> > + case AID_INET:
> > + for (i=0; i < (sizeof (icmp_type) / sizeof(icmp_type[0]));
>
> No space after sizeof, I'd drop the extra parens. Might also be worth it
> to add a local version of nitems() to this file.
This and the others are IIRC streight from pfctl. So if someone wants a
free commit :)
--
:wq Claudio
Re: llvm15: Make -mbranch-protection=bti the default
Christian Weisgerber wrote:
> Mark Kettenis:
>
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: kette...@cvs.openbsd.org2023/04/17 12:10:26
> >
> > Modified files:
> > gnu/llvm/clang/lib/Driver/ToolChains: Clang.cpp
> >
> > Log message:
> > Make -mbranch-protection=bti the default on OpenBSD.
>
> LLVM 15 reshuffled some code, so the change above no longer applies
> there. Here's my attempt to add it:
I suspect there will be a different arm64 variation of that diff coming
which tries to change the default at a lower level.
Here is the corresponding amd64 diff (an earlier version had a bug) for
in-tree clang. It changes it so BTI is enabled by default, and then adds
missing code to allow it to be turned off.
Index: gnu/llvm/clang/include/clang/Basic/CodeGenOptions.def
===
RCS file: /cvs/src/gnu/llvm/clang/include/clang/Basic/CodeGenOptions.def,v
retrieving revision 1.4
diff -u -p -u -r1.4 CodeGenOptions.def
--- gnu/llvm/clang/include/clang/Basic/CodeGenOptions.def 17 Dec 2021
14:46:43 - 1.4
+++ gnu/llvm/clang/include/clang/Basic/CodeGenOptions.def 17 Apr 2023
18:36:15 -
@@ -102,7 +102,7 @@ CODEGENOPT(InstrumentFunctionEntryBare ,
///< -finstrument-function-entry-bare is
enabled.
CODEGENOPT(CFProtectionReturn , 1, 0) ///< if -fcf-protection is
///< set to full or return.
-CODEGENOPT(CFProtectionBranch , 1, 0) ///< if -fcf-protection is
+CODEGENOPT(CFProtectionBranch , 1, 1) ///< if -fcf-protection is
///< set to full or branch.
CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is
///< enabled.
Index: gnu/llvm/clang/lib/Frontend/CompilerInvocation.cpp
===
RCS file: /cvs/src/gnu/llvm/clang/lib/Frontend/CompilerInvocation.cpp,v
retrieving revision 1.5
diff -u -p -u -r1.5 CompilerInvocation.cpp
--- gnu/llvm/clang/lib/Frontend/CompilerInvocation.cpp 17 Dec 2021 14:46:44
- 1.5
+++ gnu/llvm/clang/lib/Frontend/CompilerInvocation.cpp 18 Apr 2023 10:48:28
-
@@ -1792,7 +1792,10 @@ bool CompilerInvocation::ParseCodeGenArg
Opts.CFProtectionReturn = 1;
else if (Name == "branch")
Opts.CFProtectionBranch = 1;
-else if (Name != "none")
+else if (Name == "none") {
+ Opts.CFProtectionBranch = 0;
+ Opts.CFProtectionReturn = 0;
+} else
Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) <<
Name;
}
@@ -3653,7 +3656,8 @@ bool CompilerInvocation::ParseLangArgs(L
StringRef Name = A->getValue();
if (Name == "full" || Name == "branch") {
Opts.CFProtectionBranch = 1;
-}
+} else if (Name == "none")
+ Opts.CFProtectionBranch = 1;
}
if ((Args.hasArg(OPT_fsycl_is_device) || Args.hasArg(OPT_fsycl_is_host)) &&
xhci(4): map MSI-X
Hi,
I noticed that on Qemu with arm64 we're falling back to legacy
interrupts with xhci(4) on PCI. Turns out that the virtual xHCI
does not support MSI, but it does support MSI-X. By having xhci(4)
map MSI-X as well I can use that on Qemu.
My x395, which so far used MSI, is still working fine with this:
-xhci0 at pci6 dev 0 function 3 "AMD 17h/1xh xHCI" rev 0x00: msi, xHCI 1.10
+xhci0 at pci6 dev 0 function 3 "AMD 17h/1xh xHCI" rev 0x00: msix, xHCI 1.10
-xhci1 at pci6 dev 0 function 4 "AMD 17h/1xh xHCI" rev 0x00: msi, xHCI 1.10
+xhci1 at pci6 dev 0 function 4 "AMD 17h/1xh xHCI" rev 0x00: msix, xHCI 1.10
ok?
Patrick
diff --git a/sys/dev/pci/xhci_pci.c b/sys/dev/pci/xhci_pci.c
index 88bdf66dbd74..52d0bc1b6761 100644
--- a/sys/dev/pci/xhci_pci.c
+++ b/sys/dev/pci/xhci_pci.c
@@ -158,7 +158,8 @@ xhci_pci_attach(struct device *parent, struct device *self,
void *aux)
}
/* Map and establish the interrupt. */
- if (pci_intr_map_msi(pa, ) != 0 && pci_intr_map(pa, ) != 0) {
+ if (pci_intr_map_msix(pa, 0, ) != 0 &&
+ pci_intr_map_msi(pa, ) != 0 && pci_intr_map(pa, ) != 0) {
printf(": couldn't map interrupt\n");
goto unmap_ret;
}
Re: bgpd flowspec parser
On Tue, Apr 18, 2023 at 11:29:26AM +0200, Claudio Jeker wrote:
> This diff adds the parse.y and config.c bits for flowspec.
> I tried to make flowspec rules as similar to pf rules (even though
> flowspec is more flexible).
>
> Now this diff does nothing in itself but is already large enough to not
> add more to it. In parse.y the individual flowspec components are built up
> in a flowspec context and then at the end converted into a struct flowspec
> object. I wrapped that into a struct flowspec_config so that all the
> parent config bits can be kept together. (For struct network this is
> currently the other way around but I plan to change this at a later
> point).
ok tb
Couple of nits and comments inline. Apart from two copy-paste errors
(s/4/6) nothing important
>
> --
> :wq Claudio
>
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.471
> diff -u -p -r1.471 bgpd.h
> --- bgpd.h17 Apr 2023 08:02:21 - 1.471
> +++ bgpd.h18 Apr 2023 09:02:09 -
> @@ -231,6 +231,9 @@ SIMPLEQ_HEAD(l3vpn_head, l3vpn);
> struct network;
> TAILQ_HEAD(network_head, network);
>
> +struct flowspec_config;
> +RB_HEAD(flowspec_tree, flowspec_config);
> +
> struct prefixset;
> SIMPLEQ_HEAD(prefixset_head, prefixset);
> struct prefixset_item;
> @@ -271,6 +274,7 @@ struct roa {
> };
>
> RB_HEAD(roa_tree, roa);
> +struct aspa_set;
> RB_HEAD(aspa_tree, aspa_set);
>
> struct set_table;
> @@ -287,6 +291,7 @@ struct bgpd_config {
> struct peer_head peers;
> struct l3vpn_headl3vpns;
> struct network_head networks;
> + struct flowspec_tree flowspecs;
> struct filter_head *filters;
> struct listen_addrs *listen_addrs;
> struct mrt_head *mrt;
> @@ -514,8 +519,23 @@ struct network_config {
> };
>
> struct network {
> - struct network_config net;
> - TAILQ_ENTRY(network)entry;
> + struct network_config net;
> + TAILQ_ENTRY(network)entry;
> +};
> +
> +struct flowspec {
> + uint16_tlen;
> + uint8_t aid;
> + uint8_t pad;
> + uint8_t data[1];
> +};
> +#define FLOWSPEC_SIZE(offsetof(struct flowspec, data))
> +
> +struct flowspec_config {
> + RB_ENTRY(flowspec_config)entry;
> + struct filter_set_head attrset;
> + struct flowspec *flow;
> + enum reconf_action reconf_action;
> };
>
> enum rtr_error {
> @@ -1121,6 +1141,10 @@ extern const struct ext_comm_pairs iana_
> #define FLOWSPEC_TYPE_FLOW 13
> #define FLOWSPEC_TYPE_MAX14
>
> +#define FLOWSPEC_TCP_FLAG_STRING "FSRPAUEW"
> +#define FLOWSPEC_FRAG_STRING4"DIFL"
> +#define FLOWSPEC_FRAG_STRING6" IFL"
> +
> struct filter_prefix {
> struct bgpd_addraddr;
> uint8_t op;
> @@ -1366,6 +1390,8 @@ int control_imsg_relay(struct imsg *, st
> struct bgpd_config *new_config(void);
> void copy_config(struct bgpd_config *, struct bgpd_config *);
> void network_free(struct network *);
> +struct flowspec_config *flowspec_alloc(uint8_t, int);
> +void flowspec_free(struct flowspec_config *);
> void free_l3vpns(struct l3vpn_head *);
> void free_config(struct bgpd_config *);
> void free_prefixsets(struct prefixset_head *);
> @@ -1382,6 +1408,7 @@ voidexpand_networks(struct bgpd_config
> RB_PROTOTYPE(prefixset_tree, prefixset_item, entry, prefixset_cmp);
> RB_PROTOTYPE(roa_tree, roa, entry, roa_cmp);
> RB_PROTOTYPE(aspa_tree, aspa_set, entry, aspa_cmp);
> +RB_PROTOTYPE(flowspec_tree, flowspec_config, entry, flowspec_config_cmp);
>
> /* kroute.c */
> int kr_init(int *, uint8_t);
> Index: config.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
> retrieving revision 1.106
> diff -u -p -r1.106 config.c
> --- config.c 28 Dec 2022 21:30:15 - 1.106
> +++ config.c 18 Apr 2023 09:07:42 -
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -33,6 +34,7 @@
>
> int host_ip(const char *, struct bgpd_addr *, uint8_t *);
> void free_networks(struct network_head *);
> +void free_flowspecs(struct flowspec_tree *);
>
> struct bgpd_config *
> new_config(void)
> @@ -53,6 +55,7 @@ new_config(void)
> /* init the various list for later */
> RB_INIT(>peers);
> TAILQ_INIT(>networks);
> + RB_INIT(>flowspecs);
> SIMPLEQ_INIT(>l3vpns);
> SIMPLEQ_INIT(>prefixsets);
> SIMPLEQ_INIT(>originsets);
> @@ -105,6 +108,50
llvm15: Make -mbranch-protection=bti the default
Mark Kettenis:
> CVSROOT: /cvs
> Module name: src
> Changes by: kette...@cvs.openbsd.org2023/04/17 12:10:26
>
> Modified files:
> gnu/llvm/clang/lib/Driver/ToolChains: Clang.cpp
>
> Log message:
> Make -mbranch-protection=bti the default on OpenBSD.
LLVM 15 reshuffled some code, so the change above no longer applies
there. Here's my attempt to add it:
---
commit d80c4bf3bc72f631512bdaa922cc31d46a07257b (llvm15)
from: Christian Weisgerber
date: Mon Apr 17 22:33:20 2023 UTC
llvm: Make -mbranch-protection=bti the default on OpenBSD.
diff f0fc5537edb677b1573883cbb90bfbfe54440790
d80c4bf3bc72f631512bdaa922cc31d46a07257b
commit - f0fc5537edb677b1573883cbb90bfbfe54440790
commit + d80c4bf3bc72f631512bdaa922cc31d46a07257b
blob - 8c27644576624ffcb1242d848a9684fa16b7744c
blob + 903e477bb0e466763e5ba8e1156d763986310c81
--- gnu/llvm/clang/lib/Driver/ToolChains/Clang.cpp
+++ gnu/llvm/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1671,15 +1671,19 @@ static void CollectARMPACBTIOptions(const ToolChain
static void CollectARMPACBTIOptions(const ToolChain , const ArgList ,
ArgStringList , bool isAArch64) {
+ const llvm::Triple = TC.getEffectiveTriple();
+
const Arg *A = isAArch64
? Args.getLastArg(options::OPT_msign_return_address_EQ,
options::OPT_mbranch_protection_EQ)
: Args.getLastArg(options::OPT_mbranch_protection_EQ);
- if (!A)
+ if (!A) {
+if (isAArch64 && Triple.isOSOpenBSD())
+ CmdArgs.push_back("-mbranch-target-enforce");
return;
+ }
const Driver = TC.getDriver();
- const llvm::Triple = TC.getEffectiveTriple();
if (!(isAArch64 || (Triple.isArmT32() && Triple.isArmMClass(
D.Diag(diag::warn_incompatible_branch_protection_option)
<< Triple.getArchName();
--
Christian "naddy" Weisgerber na...@mips.inka.de
bgpd flowspec parser
This diff adds the parse.y and config.c bits for flowspec.
I tried to make flowspec rules as similar to pf rules (even though
flowspec is more flexible).
Now this diff does nothing in itself but is already large enough to not
add more to it. In parse.y the individual flowspec components are built up
in a flowspec context and then at the end converted into a struct flowspec
object. I wrapped that into a struct flowspec_config so that all the
parent config bits can be kept together. (For struct network this is
currently the other way around but I plan to change this at a later
point).
--
:wq Claudio
Index: bgpd.h
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.471
diff -u -p -r1.471 bgpd.h
--- bgpd.h 17 Apr 2023 08:02:21 - 1.471
+++ bgpd.h 18 Apr 2023 09:02:09 -
@@ -231,6 +231,9 @@ SIMPLEQ_HEAD(l3vpn_head, l3vpn);
struct network;
TAILQ_HEAD(network_head, network);
+struct flowspec_config;
+RB_HEAD(flowspec_tree, flowspec_config);
+
struct prefixset;
SIMPLEQ_HEAD(prefixset_head, prefixset);
struct prefixset_item;
@@ -271,6 +274,7 @@ struct roa {
};
RB_HEAD(roa_tree, roa);
+struct aspa_set;
RB_HEAD(aspa_tree, aspa_set);
struct set_table;
@@ -287,6 +291,7 @@ struct bgpd_config {
struct peer_head peers;
struct l3vpn_headl3vpns;
struct network_head networks;
+ struct flowspec_tree flowspecs;
struct filter_head *filters;
struct listen_addrs *listen_addrs;
struct mrt_head *mrt;
@@ -514,8 +519,23 @@ struct network_config {
};
struct network {
- struct network_config net;
- TAILQ_ENTRY(network)entry;
+ struct network_config net;
+ TAILQ_ENTRY(network)entry;
+};
+
+struct flowspec {
+ uint16_tlen;
+ uint8_t aid;
+ uint8_t pad;
+ uint8_t data[1];
+};
+#define FLOWSPEC_SIZE (offsetof(struct flowspec, data))
+
+struct flowspec_config {
+ RB_ENTRY(flowspec_config)entry;
+ struct filter_set_head attrset;
+ struct flowspec *flow;
+ enum reconf_action reconf_action;
};
enum rtr_error {
@@ -1121,6 +1141,10 @@ extern const struct ext_comm_pairs iana_
#define FLOWSPEC_TYPE_FLOW 13
#define FLOWSPEC_TYPE_MAX 14
+#define FLOWSPEC_TCP_FLAG_STRING "FSRPAUEW"
+#define FLOWSPEC_FRAG_STRING4 "DIFL"
+#define FLOWSPEC_FRAG_STRING6 " IFL"
+
struct filter_prefix {
struct bgpd_addraddr;
uint8_t op;
@@ -1366,6 +1390,8 @@ int control_imsg_relay(struct imsg *, st
struct bgpd_config *new_config(void);
void copy_config(struct bgpd_config *, struct bgpd_config *);
void network_free(struct network *);
+struct flowspec_config *flowspec_alloc(uint8_t, int);
+void flowspec_free(struct flowspec_config *);
void free_l3vpns(struct l3vpn_head *);
void free_config(struct bgpd_config *);
void free_prefixsets(struct prefixset_head *);
@@ -1382,6 +1408,7 @@ void expand_networks(struct bgpd_config
RB_PROTOTYPE(prefixset_tree, prefixset_item, entry, prefixset_cmp);
RB_PROTOTYPE(roa_tree, roa, entry, roa_cmp);
RB_PROTOTYPE(aspa_tree, aspa_set, entry, aspa_cmp);
+RB_PROTOTYPE(flowspec_tree, flowspec_config, entry, flowspec_config_cmp);
/* kroute.c */
int kr_init(int *, uint8_t);
Index: config.c
===
RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
retrieving revision 1.106
diff -u -p -r1.106 config.c
--- config.c28 Dec 2022 21:30:15 - 1.106
+++ config.c18 Apr 2023 09:07:42 -
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -33,6 +34,7 @@
inthost_ip(const char *, struct bgpd_addr *, uint8_t *);
void free_networks(struct network_head *);
+void free_flowspecs(struct flowspec_tree *);
struct bgpd_config *
new_config(void)
@@ -53,6 +55,7 @@ new_config(void)
/* init the various list for later */
RB_INIT(>peers);
TAILQ_INIT(>networks);
+ RB_INIT(>flowspecs);
SIMPLEQ_INIT(>l3vpns);
SIMPLEQ_INIT(>prefixsets);
SIMPLEQ_INIT(>originsets);
@@ -105,6 +108,50 @@ free_networks(struct network_head *netwo
}
}
+struct flowspec_config *
+flowspec_alloc(uint8_t aid, int len)
+{
+ struct flowspec_config *conf;
+ struct flowspec *flow;
+
+ flow = malloc(FLOWSPEC_SIZE + len);
+ if (flow == NULL)
+ return NULL;
+ memset(flow, 0, FLOWSPEC_SIZE);
+
+ conf = calloc(1,
Re: unwind(8): fix (some?) bad packet log messages
On Sat, Apr 15, 2023 at 12:11:28PM +0200, Florian Obser wrote:
> Turns out I found a way to trigger "bad packet: too short" messages by
> suspending / resuming my laptop in just the right spot. That allowed me
> to figure out what's going on:
> When asr fails (i.e. the stub strategy) it obviously does not synthesize
> a SERVFAIL DNS packet for us like libunbound does. resolve_done() and
> check_resolver_done() however depended on this. Since we are not looking
> at the packet in case of SERVFAIL there is no need to check the packet
> size and then complain about this, we can just bail out early.
>
> So we first need to fix that asr errors are propagated up as rcode
> SERVFAIL and then use the rcode as the first check in resolve_done() /
> check_resolver_done().
>
> I'm not sure where the "too long" errors are coming from, I suspect
> there is an error path in libunbound that reports a wrong length because
> it doesn't reset the internal sldns_buffer. I'm cautiously optimistic
> that those errors are also caught by this diff because rcode might just
> be set correctly.
>
> OK?
it reduces the amount of log messages here (I have a gateway which reject
traffic based on the clock).
it reads fine. ok semarie@
> commit c25ea4620c5ed21a5d12556fafba1f1ac22842e3
> Author: Florian Obser
> Date: Sat Apr 15 11:41:42 2023 +0200
>
> Improve asr error handling.
>
> When an upstream nameserver is not available asr is not synthesizing a
> SERVFAIL rcode (duh), but sets ar_errno. When we need SERVFAIL we need
> to set it ourselves.
>
> While here, don't complain about a too short packet when asr already
> told us that resolving did not work out in check_dns64_done.
>
> diff --git resolver.c resolver.c
> index 71614237506..9b60445f80d 100644
> --- resolver.c
> +++ resolver.c
> @@ -1623,8 +1623,9 @@ void
> asr_resolve_done(struct asr_result *ar, void *arg)
> {
> struct resolver_cb_data *cb_data = arg;
> - cb_data->cb(cb_data->res, cb_data->data, ar->ar_rcode, ar->ar_data,
> - ar->ar_datalen, 0, NULL);
> + cb_data->cb(cb_data->res, cb_data->data, ar->ar_errno == 0 ?
> + ar->ar_rcode : LDNS_RCODE_SERVFAIL, ar->ar_data, ar->ar_datalen, 0,
> + NULL);
> free(ar->ar_data);
> resolver_unref(cb_data->res);
> free(cb_data);
> @@ -2289,6 +2290,9 @@ check_dns64_done(struct asr_result *ar, void *arg)
> int preflen, count = 0;
> void*asr_ctx = arg;
>
> + if (ar->ar_errno != 0)
> + goto fail;
> +
> memset(, 0, sizeof(qinfo));
> alloc_init(, NULL, 0);
>
> @@ -2390,6 +2394,7 @@ check_dns64_done(struct asr_result *ar, void *arg)
> alloc_clear();
> regional_destroy(region);
> sldns_buffer_free(buf);
> + fail:
> free(ar->ar_data);
> asr_resolver_free(asr_ctx);
> }
>
> commit 9ede59d6a547b0e5f574819bb7fcaf68eda24630
> Author: Florian Obser
> Date: Sat Apr 15 11:46:40 2023 +0200
>
> If rcode is SERVFAIL, there is no need to look at the packet.
>
> This pulls the check for rcode up, before we check if the answer
> packet has sensible length. Since we are not touching the packet at
> all, we don't care about the size and don't need to log if the size is
> wrong from a DNS perspective.
>
> With asr error reporting improved in the previous commit, this
> probably gets rid of all "bad packet: too short" messages.
>
> diff --git resolver.c resolver.c
> index 9b60445f80d..9625dcb471a 100644
> --- resolver.c
> +++ resolver.c
> @@ -953,6 +953,12 @@ resolve_done(struct uw_resolver *res, void *arg, int
> rcode,
>
> running_res = --rq->running;
>
> + if (rcode == LDNS_RCODE_SERVFAIL) {
> + if (res->stop != 1)
> + check_resolver(res);
> + goto servfail;
> + }
> +
> if (answer_len < LDNS_HEADER_SIZE) {
> log_warnx("bad packet: too short");
> goto servfail;
> @@ -965,12 +971,6 @@ resolve_done(struct uw_resolver *res, void *arg, int
> rcode,
> }
> answer_header->answer_len = answer_len;
>
> - if (rcode == LDNS_RCODE_SERVFAIL) {
> - if (res->stop != 1)
> - check_resolver(res);
> - goto servfail;
> - }
> -
> if ((result = calloc(1, sizeof(*result))) == NULL)
> goto servfail;
> if ((buf = sldns_buffer_new(answer_len)) == NULL)
> @@ -1545,12 +1545,6 @@ check_resolver_done(struct uw_resolver *res, void
> *arg, int rcode,
>
> prev_state = checked_resolver->state;
>
> - if (answer_len < LDNS_HEADER_SIZE) {
> - checked_resolver->state = DEAD;
> - log_warnx("%s: bad packet: too short", __func__);
> - goto out;
> - }
> -
> if (rcode == LDNS_RCODE_SERVFAIL) {
> log_debug("%s: %s rcode: SERVFAIL", __func__,
>
