Re: Fix -Wincompatible-pointer-types-discards-qualifiers
Thanks so much! This is my first patch for OpenBSD, and I don't quite have
the workflow debugged yet.
Adam
On Thu, Jan 7, 2021 at 11:29 PM Theo Buehler wrote:
> On Thu, Jan 07, 2021 at 11:16:16PM +, Adam Barth wrote:
> > Previously, this code was passing string constants to functions that did
> > not declare their parameters as const. After this patch, the functions
> now
> > declare that they do not modify these arguments, making it safe to pass
> > string constants.
>
> Thanks. Unfortunately the patch was mangled by your MUA (line wrapping
> and expanded tabs).
>
> Below is a version that applies.
>
> ok tb
>
> Index: lib/libc/gen/fts.c
> ===
> RCS file: /cvs/src/lib/libc/gen/fts.c,v
> retrieving revision 1.59
> diff -u -p -r1.59 fts.c
> --- lib/libc/gen/fts.c 28 Jun 2019 13:32:41 - 1.59
> +++ lib/libc/gen/fts.c 7 Jan 2021 23:23:27 -
> @@ -43,7 +43,7 @@
>
> #define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
>
> -static FTSENT *fts_alloc(FTS *, char *, size_t);
> +static FTSENT *fts_alloc(FTS *, const char *, size_t);
> static FTSENT *fts_build(FTS *, int);
> static void fts_lfree(FTSENT *);
> static void fts_load(FTS *, FTSENT *);
> @@ -52,7 +52,7 @@ static voidfts_padjust(FTS *, FTSENT *
> static int fts_palloc(FTS *, size_t);
> static FTSENT *fts_sort(FTS *, FTSENT *, int);
> static u_short fts_stat(FTS *, FTSENT *, int, int);
> -static int fts_safe_changedir(FTS *, FTSENT *, int, char *);
> +static int fts_safe_changedir(FTS *, FTSENT *, int, const char *);
>
> #defineISDOT(a)(a[0] == '.' && (!a[1] || (a[1] == '.' &&
> !a[2])))
>
> @@ -901,7 +901,7 @@ fts_sort(FTS *sp, FTSENT *head, int nite
> }
>
> static FTSENT *
> -fts_alloc(FTS *sp, char *name, size_t namelen)
> +fts_alloc(FTS *sp, const char *name, size_t namelen)
> {
> FTSENT *p;
> size_t len;
> @@ -1020,7 +1020,7 @@ fts_maxarglen(char * const *argv)
> * Assumes p->fts_dev and p->fts_ino are filled in.
> */
> static int
> -fts_safe_changedir(FTS *sp, FTSENT *p, int fd, char *path)
> +fts_safe_changedir(FTS *sp, FTSENT *p, int fd, const char *path)
> {
> int ret, oerrno, newfd;
> struct stat sb;
>
Fix -Wincompatible-pointer-types-discards-qualifiers
Previously, this code was passing string constants to functions that did
not declare their parameters as const. After this patch, the functions now
declare that they do not modify these arguments, making it safe to pass
string constants.
diff --git lib/libc/gen/fts.c lib/libc/gen/fts.c
index d13d0a3f223..dd8f65b02da 100644
--- lib/libc/gen/fts.c
+++ lib/libc/gen/fts.c
@@ -43,7 +43,7 @@
#define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
-static FTSENT *fts_alloc(FTS *, char *, size_t);
+static FTSENT *fts_alloc(FTS *, const char *, size_t);
static FTSENT *fts_build(FTS *, int);
static void fts_lfree(FTSENT *);
static void fts_load(FTS *, FTSENT *);
@@ -52,7 +52,7 @@ static voidfts_padjust(FTS *, FTSENT *);
static int fts_palloc(FTS *, size_t);
static FTSENT *fts_sort(FTS *, FTSENT *, int);
static u_short fts_stat(FTS *, FTSENT *, int, int);
-static int fts_safe_changedir(FTS *, FTSENT *, int, char *);
+static int fts_safe_changedir(FTS *, FTSENT *, int, const char *);
#defineISDOT(a)(a[0] == '.' && (!a[1] || (a[1] == '.' &&
!a[2])))
@@ -901,7 +901,7 @@ fts_sort(FTS *sp, FTSENT *head, int nitems)
}
static FTSENT *
-fts_alloc(FTS *sp, char *name, size_t namelen)
+fts_alloc(FTS *sp, const char *name, size_t namelen)
{
FTSENT *p;
size_t len;
@@ -1020,7 +1020,7 @@ fts_maxarglen(char * const *argv)
* Assumes p->fts_dev and p->fts_ino are filled in.
*/
static int
-fts_safe_changedir(FTS *sp, FTSENT *p, int fd, char *path)
+fts_safe_changedir(FTS *sp, FTSENT *p, int fd, const char *path)
{
int ret, oerrno, newfd;
struct stat sb;
Re: all platforms: isolate hardclock(9) from statclock()
On Thu, Jan 07, 2021 at 09:37:58PM +0100, Mark Kettenis wrote:
> > Date: Thu, 7 Jan 2021 11:15:41 -0600
> > From: Scott Cheloha
> >
> > Hi,
> >
> > I want to isolate statclock() from hardclock(9). This will simplify
> > the logic in my WIP dynamic clock interrupt framework.
> >
> > Currently, if stathz is zero, we call statclock() from within
> > hardclock(9). It looks like this (see sys/kern/kern_clock.c):
> >
> > void
> > hardclock(struct clockframe *frame)
> > {
> > /* [...] */
> >
> > if (stathz == 0)
> > statclock(frame);
> >
> > /* [...] */
> >
> > This is the case on alpha, amd64 (w/ lapic), hppa, i386 (w/ lapic),
> > loongson, luna88k, mips64, and sh.
> >
> > (We seem to do it on sgi, too. I was under the impression that sgi
> > *was* a mips64 platform, yet sgi seems to it have its own clock
> > interrupt code distinct from mips64's general clock interrupt code
> > which is used by e.g. octeon).
> >
> > However, if stathz is not zero we call statclock() separately. This
> > is the case on armv7, arm, arm64, macppc, powerpc64, and sparc64.
> >
> > (The situation for the general powerpc code and socppc in particular
> > is a mystery to me.)
> >
> > If we could remove this MD distinction it would make my MI framework
> > simpler. Instead of checking stathz and conditionally starting a
> > statclock event I will be able to unconditionally start a statclock
> > event on all platforms on every CPU.
> >
> > In general I don't think the "is stathz zero?" variance between
> > platforms is useful:
> >
> > - The difference is invisible to userspace, as we hide the fact that
> > stathz is zero from e.g. the kern.clockrate sysctl.
> >
> > - We run statclock() at some point, regardless of whether stathz is
> > zero. If statclock() is run from hardclock(9) then isn't stathz
> > effectively equal to hz?
> >
> > - Because stathz might be zero we need to add a bunch of safety checks
> > to our MI code to ensure we don't accidentally divide by zero.
> >
> > Maybe we can ensure stathz is non-zero in a later diff...
> >
> > --
> >
> > Anyway, I don't think I have missed any platforms. However, if
> > platform experts could weigh in here to verify my changes (and test
> > them!) I'd really appreciate it.
> >
> > In particular, I'm confused about how clock interrupts work on
> > powerpc, socppc, and sgi.
> >
> > --
> >
> > Thoughts? Platform-specific OKs?
>
> I wouldn't be opposed to doing this. It is less magic!
>
> But yes, this needs to be tested on the platforms that you change.
I guess I'll CC all the platform-specific people I'm aware of.
> Note that many platforms don't have have separate schedclock and
> statclock. But on many platforms where we use a one-shot timer as the
> clock we have a randomized statclock. I'm sure Theo would love to
> tell you about the cpuhog story...
I am familiar with cpuhog. It's the one thing everybody mentions when
I talk about clock interrupts and/or statclock().
Related:
I wonder if we could avoid the cpuhog problem entirely by implementing
some kind of MI cycle counting clock API that we use to timestamp
whenever we cross the syscall boundary, or enter an interrupt, etc.,
to determine the time a thread spends using the CPU without any
sampling error.
Instead of a process accumulating ticks from a sampling clock
interrupt you would accumulate, say, a 64-bit count of cycles, or
something like that.
Sampling with a regular clock interrupt is prone to error and trickery
like cpuhog. The classic BSD solution to the cpuhog exploit was to
randomize the statclock/schedclock to make it harder to fool the
sampler. But if we used cycle counts or instruction counts at each
state transition it would be impossible to fool because we wouldn't be
sampling at all.
Unsure what the performance implications would be, but in general I
would guess that most platforms have a way to count instructions or
cycles and that reading this data is fast enough for us to use it in
e.g. syscall() or the interrupt handler without a huge performance
hit.
> Anyway, we probably want that on amd64 as well.
My WIP dynamic clock interrupt system can run a randomized statclock()
on amd64 boxes with a lapic. I imagine we will be able to do the same
on i386 systems that have a lapic, too, though it will be slower
because all the i386 timecounters are glacial compared to the TSC.
Eventually I want to isolate schedclock() from statclock() and run it
as an independent event. But that's a "later on" goal. For now I'm
just trying to get every platform as similar as possible to make
merging the dynamic clock interrupt work less painful.
Index: sys/kern/kern_clock.c
===
RCS file: /cvs/src/sys/kern/kern_clock.c,v
retrieving revision 1.101
diff -u -p -r1.101 kern_clock.c
--- sys/kern/kern_clock.c 21 Jan 2020 16:16:23 - 1.101
+++ sys/kern/kern_clock.c 7 Jan 2021 16:3
Re: Fix -Wincompatible-pointer-types-discards-qualifiers
On Fri, 08 Jan 2021 00:29:32 +0100, Theo Buehler wrote: > Thanks. Unfortunately the patch was mangled by your MUA (line wrapping > and expanded tabs). > > Below is a version that applies. OK millert@ as well. - todd
Re: Fix -Wincompatible-pointer-types-discards-qualifiers
On Thu, Jan 07, 2021 at 11:16:16PM +, Adam Barth wrote:
> Previously, this code was passing string constants to functions that did
> not declare their parameters as const. After this patch, the functions now
> declare that they do not modify these arguments, making it safe to pass
> string constants.
Thanks. Unfortunately the patch was mangled by your MUA (line wrapping
and expanded tabs).
Below is a version that applies.
ok tb
Index: lib/libc/gen/fts.c
===
RCS file: /cvs/src/lib/libc/gen/fts.c,v
retrieving revision 1.59
diff -u -p -r1.59 fts.c
--- lib/libc/gen/fts.c 28 Jun 2019 13:32:41 - 1.59
+++ lib/libc/gen/fts.c 7 Jan 2021 23:23:27 -
@@ -43,7 +43,7 @@
#define MAXIMUM(a, b) (((a) > (b)) ? (a) : (b))
-static FTSENT *fts_alloc(FTS *, char *, size_t);
+static FTSENT *fts_alloc(FTS *, const char *, size_t);
static FTSENT *fts_build(FTS *, int);
static void fts_lfree(FTSENT *);
static void fts_load(FTS *, FTSENT *);
@@ -52,7 +52,7 @@ static voidfts_padjust(FTS *, FTSENT *
static int fts_palloc(FTS *, size_t);
static FTSENT *fts_sort(FTS *, FTSENT *, int);
static u_short fts_stat(FTS *, FTSENT *, int, int);
-static int fts_safe_changedir(FTS *, FTSENT *, int, char *);
+static int fts_safe_changedir(FTS *, FTSENT *, int, const char *);
#defineISDOT(a)(a[0] == '.' && (!a[1] || (a[1] == '.' &&
!a[2])))
@@ -901,7 +901,7 @@ fts_sort(FTS *sp, FTSENT *head, int nite
}
static FTSENT *
-fts_alloc(FTS *sp, char *name, size_t namelen)
+fts_alloc(FTS *sp, const char *name, size_t namelen)
{
FTSENT *p;
size_t len;
@@ -1020,7 +1020,7 @@ fts_maxarglen(char * const *argv)
* Assumes p->fts_dev and p->fts_ino are filled in.
*/
static int
-fts_safe_changedir(FTS *sp, FTSENT *p, int fd, char *path)
+fts_safe_changedir(FTS *sp, FTSENT *p, int fd, const char *path)
{
int ret, oerrno, newfd;
struct stat sb;
Re: syncer_thread: sleep without lbolt
On Sat, Dec 12, 2020 at 01:32:13PM -0600, Scott Cheloha wrote:
> Hi,
>
> The syncer thread is one of the last users of the lbolt (lightning
> bolt!) sleep channel.
>
> If we add a syncer-specific sleep channel (syncer_chan) and do a bit
> of time math we can replicate the current behavior and remove another
> lbolt user.
>
> This isn't a perfect recreation of the current behavior. In this
> version the sleep period will drift if processing takes longer than 1
> second. I think it's good enough. If people are concerned about a
> perfect recreation of the current behavior we *can* do it, but it will
> require more code. I don't think it's worth it.
>
> This also fixes two problems in the current code. They aren't huge
> bugs, but they did jump out as potential problems because they make
> the syncer's behavior less deterministic:
>
> - The current code uses gettime(9), which will jump and screw up your
> measurement if someone calls settimeofday(2). The new code uses the
> uptime clock, which is monotonic and stable.
>
> - The current code uses gettime(9), which has a resolution of 1
> second. Measuring a 1 second timeout with an interface with
> a resolution of 1 second is crude and error-prone. The new code
> uses getnsecuptime(), which has a resolution of roughly 1/hz.
> Much better.
>
> I vaguely recall beck@ trying to do something with this in the recent
> past, so CC beck@.
1-ish month bump.
Index: vfs_sync.c
===
RCS file: /cvs/src/sys/kern/vfs_sync.c,v
retrieving revision 1.64
diff -u -p -r1.64 vfs_sync.c
--- vfs_sync.c 24 Jun 2020 22:03:41 - 1.64
+++ vfs_sync.c 25 Dec 2020 17:09:00 -
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
@@ -73,6 +74,7 @@ LIST_HEAD(synclist, vnode);
static struct synclist *syncer_workitem_pending;
struct proc *syncerproc;
+int syncer_chan;
/*
* The workitem queue.
@@ -130,6 +132,19 @@ vn_syncer_add_to_worklist(struct vnode *
}
/*
+ * TODO Move getnsecuptime() to kern_tc.c and document it when we have
+ * more users in the kernel.
+ */
+static uint64_t
+getnsecuptime(void)
+{
+ struct timespec now;
+
+ getnanouptime(&now);
+ return TIMESPEC_TO_NSEC(&now);
+}
+
+/*
* System filesystem synchronizer daemon.
*/
void
@@ -138,11 +153,11 @@ syncer_thread(void *arg)
struct proc *p = curproc;
struct synclist *slp;
struct vnode *vp;
- time_t starttime;
+ uint64_t elapsed, start;
int s;
for (;;) {
- starttime = gettime();
+ start = getnsecuptime();
/*
* Push files whose dirty time has expired.
@@ -220,6 +235,7 @@ syncer_thread(void *arg)
rushjob -= 1;
continue;
}
+
/*
* If it has taken us less than a second to process the
* current work, then wait. Otherwise start right over
@@ -228,8 +244,11 @@ syncer_thread(void *arg)
* matter as we are just trying to generally pace the
* filesystem activity.
*/
- if (gettime() == starttime)
- tsleep_nsec(&lbolt, PPAUSE, "syncer", INFSLP);
+ elapsed = getnsecuptime() - start;
+ if (elapsed < SEC_TO_NSEC(1)) {
+ tsleep_nsec(&syncer_chan, PPAUSE, "syncer",
+ SEC_TO_NSEC(1) - elapsed);
+ }
}
}
@@ -242,7 +261,7 @@ int
speedup_syncer(void)
{
if (syncerproc)
- wakeup_proc(syncerproc, &lbolt);
+ wakeup_proc(syncerproc, &syncer_chan);
if (rushjob < syncdelay / 2) {
rushjob += 1;
stat_rush_requests += 1;
Re: pf route-to issues
Hello, sorry to come back after while... On Tue, Jan 05, 2021 at 10:05:39PM +1000, David Gwynne wrote: > On Mon, Jan 04, 2021 at 06:37:54PM +0100, Alexander Bluhm wrote: > > On Mon, Jan 04, 2021 at 11:21:50PM +1000, David Gwynne wrote: > > > this chunk pops out as a standalone change. > > > > > > having pf_find_state() return PF_PASS here means the callers short > > > circuit and let the packet go through without running it through the > > > a lot of the state handling, which includes things like protocol state > > > updates, nat, scrubbing, some pflog handling, and most importantly, > > > later calls to pf_route(). > > > > pf_route() calls pf_test() again with a different interface. > > > > The idea of this code is, that the interface which is passed to > > pf_test() from ip_output() is wrong. The call to pf_set_rt_ifp() > > changes it in the state. > > > > In the pf_test() call from ip_output() we skip the tests. We know > > they will happen in pf_test() called from pf_route(). Without this > > chunk we would do state handling twice with different interfaces. > > > > Is that analysis correct? > > I think so, but I didn't get as much time to poke at this today as I was > hoping. > > If the idea is to avoid running most of pf_test again if route-to is > applied during ip_output, I think this tweaked diff is simpler. Is there > a valid use case for running some of pf_test again after route-to is > applied? > > The pf_set_rt_ifp() stuff could be cleaned up if we can get away with > this. I think this should go in. I was trying to test this change, but I'm unable to create set up which would work on google cloud in the way I need. I suspect failing to convince underlying vswitch to carry packets between test hosts for me. as soon as I start to do something more fancy, packets start to disappear in underlying fabric... If I understand the change right we really need to take care of route-to action for inbound packet 'to convince PF' stack the packet actually enters firewall on interface desired by route-to action ordered by rule found by currently executed pf_test(). for outbound case the IP stack will be executing the pf_test(). so yes, I'm OK with this change. regards sashan
Re: extend ip(4) to document ip_mreqn
On Thu, Jan 07, 2021 at 04:03:41PM +0100, Claudio Jeker wrote:
> Here is my try to extend ip(4) to also document struct ip_mreqn.
> Not sure what is the best way to document the option to use either struct
> ip_mreq or struct ip_mreqn with IP_ADD_MEMBERSHIP.
>
> --
> :wq Claudio
>
hi! comments inline:
> Index: ip.4
> ===
> RCS file: /cvs/src/share/man/man4/ip.4,v
> retrieving revision 1.41
> diff -u -p -r1.41 ip.4
> --- ip.4 18 Aug 2016 11:45:18 - 1.41
> +++ ip.4 7 Jan 2021 14:58:54 -
> @@ -411,13 +411,21 @@ setsockopt(s, IPPROTO_IP, IP_ADD_MEMBERS
> .Pp
> where
> .Fa mreq
> -is the following structure:
> +is either the following structure:
we could probably tidy this up:
...
is either of the following structures:
.Bd ...
.Ed
> .Bd -literal -offset indent
> struct ip_mreq {
> struct in_addr imr_multiaddr; /* multicast group to join */
> struct in_addr imr_interface; /* interface to join on */
> }
> .Ed
> +or
> +.Bd -literal -offset indent
> +struct ip_mreqn {
> +struct in_addr imr_multiaddr; /* multicast group to join */
> +struct in_addr imr_address; /* local IP address of interface */
> +intimr_ifindex; /* interface index to join*/
space between "join" and "*"
> +};
> +.Ed
> .Pp
> .Va imr_interface
> should
> @@ -428,6 +436,12 @@ or the
> .Tn IP
> address of a particular multicast-capable interface if
> the host is multihomed.
> +.Va imr_ifindex
> +of
> +.Va struct ip_mreqn
this reads funny. maybe
The
.Va imr..
element of
.Va struct...
or similar wording
> +can be set to the interface index instead of specifying the
> +.Tn IP
> +address of a particular multicast-capable interface.
> Membership is associated with a single interface;
> programs running on multihomed hosts may need to
> join the same group on more than one interface.
>
jmc
Re: all platforms: isolate hardclock(9) from statclock()
> Date: Thu, 7 Jan 2021 11:15:41 -0600
> From: Scott Cheloha
>
> Hi,
>
> I want to isolate statclock() from hardclock(9). This will simplify
> the logic in my WIP dynamic clock interrupt framework.
>
> Currently, if stathz is zero, we call statclock() from within
> hardclock(9). It looks like this (see sys/kern/kern_clock.c):
>
> void
> hardclock(struct clockframe *frame)
> {
> /* [...] */
>
> if (stathz == 0)
> statclock(frame);
>
> /* [...] */
>
> This is the case on alpha, amd64 (w/ lapic), hppa, i386 (w/ lapic),
> loongson, luna88k, mips64, and sh.
>
> (We seem to do it on sgi, too. I was under the impression that sgi
> *was* a mips64 platform, yet sgi seems to it have its own clock
> interrupt code distinct from mips64's general clock interrupt code
> which is used by e.g. octeon).
>
> However, if stathz is not zero we call statclock() separately. This
> is the case on armv7, arm, arm64, macppc, powerpc64, and sparc64.
>
> (The situation for the general powerpc code and socppc in particular
> is a mystery to me.)
>
> If we could remove this MD distinction it would make my MI framework
> simpler. Instead of checking stathz and conditionally starting a
> statclock event I will be able to unconditionally start a statclock
> event on all platforms on every CPU.
>
> In general I don't think the "is stathz zero?" variance between
> platforms is useful:
>
> - The difference is invisible to userspace, as we hide the fact that
> stathz is zero from e.g. the kern.clockrate sysctl.
>
> - We run statclock() at some point, regardless of whether stathz is
> zero. If statclock() is run from hardclock(9) then isn't stathz
> effectively equal to hz?
>
> - Because stathz might be zero we need to add a bunch of safety checks
> to our MI code to ensure we don't accidentally divide by zero.
>
> Maybe we can ensure stathz is non-zero in a later diff...
>
> --
>
> Anyway, I don't think I have missed any platforms. However, if
> platform experts could weigh in here to verify my changes (and test
> them!) I'd really appreciate it.
>
> In particular, I'm confused about how clock interrupts work on
> powerpc, socppc, and sgi.
>
> --
>
> Thoughts? Platform-specific OKs?
I wouldn't be opposed to doing this. It is less magic!
But yes, this needs to be tested on the platforms that you change.
Note that many platforms don't have have separate schedclock and
statclock. But on many platforms where we use a one-shot timer as the
clock we have a randomized statclock. I'm sure Theo would love to
tell you about the cpuhog story...
Anyway, we probably want that on amd64 as well.
> Index: sys/kern/kern_clock.c
> ===
> RCS file: /cvs/src/sys/kern/kern_clock.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 kern_clock.c
> --- sys/kern/kern_clock.c 21 Jan 2020 16:16:23 - 1.101
> +++ sys/kern/kern_clock.c 7 Jan 2021 16:37:09 -
> @@ -164,12 +164,6 @@ hardclock(struct clockframe *frame)
> }
> }
>
> - /*
> - * If no separate statistics clock is available, run it from here.
> - */
> - if (stathz == 0)
> - statclock(frame);
> -
> if (--ci->ci_schedstate.spc_rrticks <= 0)
> roundrobin(ci);
>
> Index: sys/arch/alpha/alpha/clock.c
> ===
> RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 clock.c
> --- sys/arch/alpha/alpha/clock.c 6 Jul 2020 13:33:06 - 1.24
> +++ sys/arch/alpha/alpha/clock.c 7 Jan 2021 16:37:09 -
> @@ -136,6 +136,13 @@ clockattach(dev, fns)
> * Machine-dependent clock routines.
> */
>
> +void
> +clockintr(struct clockframe *frame)
> +{
> + hardclock(frame);
> + statclock(frame);
> +}
> +
> /*
> * Start the real-time and statistics clocks. Leave stathz 0 since there
> * are no other timers available.
> @@ -165,7 +172,7 @@ cpu_initclocks(void)
>* hardclock, which would then fall over because the pointer
>* to the virtual timers wasn't set at that time.
>*/
> - platform.clockintr = hardclock;
> + platform.clockintr = clockintr;
> schedhz = 16;
>
> evcount_attach(&clk_count, "clock", &clk_irq);
> Index: sys/arch/amd64/amd64/lapic.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 lapic.c
> --- sys/arch/amd64/amd64/lapic.c 6 Sep 2020 20:50:00 - 1.57
> +++ sys/arch/amd64/amd64/lapic.c 7 Jan 2021 16:37:09 -
> @@ -452,6 +452,7 @@ lapic_clockintr(void *arg, struct intrfr
> floor = ci->ci_handled_intr_level;
> ci->ci_handled_intr_level = ci->ci_ilevel;
> hardclock((struct clockframe *)&frame);
> + statclock((struct clockframe *)&frame);
> ci->ci_
Re: pair: send without kernel lock
> On 7 Jan 2021, at 04:14, Klemens Nanni wrote:
>
> pair(4)'s output path can run without kernel lock just fine.
>
> NB: Looking at CVS log, it seems this was not done during import because
> IFXF_MPSSAFE became a thing afterwards.
>
> Feedback? Objections? OK?
>
Looks good to me.
ok mvs
> Index: if_pair.c
> ===
> RCS file: /cvs/src/sys/net/if_pair.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 if_pair.c
> --- if_pair.c 21 Aug 2020 22:59:27 - 1.16
> +++ if_pair.c 7 Jan 2021 00:20:20 -
> @@ -37,7 +37,7 @@
>
> void pairattach(int);
> int pairioctl(struct ifnet *, u_long, caddr_t);
> -void pairstart(struct ifnet *);
> +void pairstart(struct ifqueue *);
> int pair_clone_create(struct if_clone *, int);
> int pair_clone_destroy(struct ifnet *);
> int pair_media_change(struct ifnet *);
> @@ -116,8 +116,8 @@ pair_clone_create(struct if_clone *ifc,
>
> ifp->if_softc = sc;
> ifp->if_ioctl = pairioctl;
> - ifp->if_start = pairstart;
> - ifp->if_xflags = IFXF_CLONED;
> + ifp->if_qstart = pairstart;
> + ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
>
> ifp->if_hardmtu = ETHER_MAX_HARDMTU_LEN;
> ifp->if_capabilities = IFCAP_VLAN_MTU;
> @@ -158,8 +158,9 @@ pair_clone_destroy(struct ifnet *ifp)
> }
>
> void
> -pairstart(struct ifnet *ifp)
> +pairstart(struct ifqueue *ifq)
> {
> + struct ifnet*ifp = ifq->ifq_if;
> struct pair_softc *sc = (struct pair_softc *)ifp->if_softc;
> struct mbuf_list ml = MBUF_LIST_INITIALIZER();
> struct ifnet*pairedifp;
> @@ -167,11 +168,7 @@ pairstart(struct ifnet *ifp)
>
> pairedifp = if_get(sc->sc_pairedif);
>
> - for (;;) {
> - m = ifq_dequeue(&ifp->if_snd);
> - if (m == NULL)
> - break;
> -
> + while ((m = ifq_dequeue(ifq)) != NULL) {
> #if NBPFILTER > 0
> if (ifp->if_bpf)
> bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_OUT);
>
Re: New ujoy(4) device for USB gamecontrollers
On Thu, 7 Jan 2021 10:20:34 -0700 Thomas Frohwein wrote: > On Wed, Jan 06, 2021 at 10:48:58PM +0100, Marcus Glocker wrote: > > [...] > > > The implementation as such looks fine to me. > > But I quickly gave the diff a spin before on amd64 using my PS > > controller, and the result is not what I would expect. > > > > With uhid, I can access the controller on /dev/uhid6. The attach > > looks like this: > > > > imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony > > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 > > rec, 4 ctls > > imac /bsd: audio1 at uaudio0 > > imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony > > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > imac /bsd: uhidev6: iclass 3/0, 180 report ids > > imac /bsd: uhid6 at uhidev6 reportid 1: input=63, output=0, > > feature=0 > > [...] > > > imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, > > feature=63 imac /bsd: uhid51 at uhidev6 reportid 180: input=0, > > output=0, feature=63 > > > > ujoy instead attached to previous uhid51, and there it is of no use > > obviously. I still can access the controller through uhid6. The > > attach with ujoy looks like this: > > > > imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony > > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 > > rec, 4 ctls > > imac /bsd: audio1 at uaudio0 > > imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony > > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > > imac /bsd: uhidev6: iclass 3/0, 180 report ids > > [...] > > > imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, > > feature=63 imac /bsd: ujoy0 at uhidev6 reportid 180: input=0, > > output=0, feature=63 > > > > I haven't analyzed yet what happens in the code. > > I can provide an lsusb of the controller if it's more obvious to > > you. > > I have heard from others who tried the diff that the PS4 controller is > causing problems with the way it attaches. I ordered one to trial-and- > error this myself at home. Could you share output of lsusb -vv? Thanks > for giving it a try! Sure, here we go. If I can find anything myself in the meantime I let you know. Bus 001 Device 006: ID 054c:09cc Sony Corp. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x054c Sony Corp. idProduct 0x09cc bcdDevice1.00 iManufacturer 1 Sony Interactive Entertainment iProduct2 Wireless Controller iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 225 bNumInterfaces 4 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 1 Audio bInterfaceSubClass 1 Control Device bInterfaceProtocol 0 iInterface 0 AudioControl Interface Descriptor: bLength10 bDescriptorType36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 wTotalLength 71 bInCollection 2 baInterfaceNr( 0) 1 baInterfaceNr( 1) 2 AudioControl Interface Descriptor: bLength12 bDescriptorType36 bDescriptorSubtype 2 (INPUT_TERMINAL) bTerminalID 1 wTerminalType 0x0101 USB Streaming bAssocTerminal 6 bNrChannels 2 wChannelConfig 0x0003 Left Front (L) Right Front (R) iChannelNames 0 iTerminal 0 AudioControl Interface Descriptor: bLength10 bDescriptorType36 bDescriptorSubtype 6 (FEATURE_UNIT) bUnitID 2 bSourceID 1 bControlSize1 bmaControls( 0) 0x03 Mute Control Volume Control bmaControls( 1) 0x00 bmaControls( 2) 0x00 iFeature0 AudioControl Interface Descriptor: bLength 9 bDescriptorType36 bDescriptorSubtype 3 (OUTPUT_TERMINAL)
bgpd simplify update path
When bgpd generates an UPDATE to update or withdraw prefixes it does this
from rde_generate_updates() and then decends into up_generate_update().
Now there is up_test_update() that checks if a new prefix is actually OK
to be distributed. It checks things for route reflectors and the common
communities (NO_EXPORT, ...). There are a few more checks that are pure
peer config checks and those should be moved up to rde_generate_updates().
Last but not least there is this bit about ORIGINATOR_ID which seems
sensible but on second thought I think it is actually wrong and an
extension on top of the RFC. Since I think this code currently has not the
right withdraw behaviour I decided it is the best to just remove it.
This code simplifies the return of up_test_update() to a pure true / false
case and make up_generate_update() simpler. Also I think doing the peer
checks early on will improve performance.
Please review :)
--
:wq Claudio
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.510
diff -u -p -r1.510 rde.c
--- rde.c 30 Dec 2020 07:29:56 - 1.510
+++ rde.c 7 Jan 2021 17:04:53 -
@@ -2814,7 +2814,8 @@ rde_send_kroute(struct rib *rib, struct
void
rde_generate_updates(struct rib *rib, struct prefix *new, struct prefix *old)
{
- struct rde_peer *peer;
+ struct rde_peer *peer;
+ u_int8_t aid;
/*
* If old is != NULL we know it was active and should be removed.
@@ -2824,6 +2825,11 @@ rde_generate_updates(struct rib *rib, st
if (old == NULL && new == NULL)
return;
+ if (new)
+ aid = new->pt->aid;
+ else
+ aid = old->pt->aid;
+
LIST_FOREACH(peer, &peerlist, peer_l) {
if (peer->conf.id == 0)
continue;
@@ -2831,6 +2837,14 @@ rde_generate_updates(struct rib *rib, st
continue;
if (peer->state != PEER_UP)
continue;
+ /* check if peer actually supports the address family */
+ if (peer->capa.mp[aid] == 0)
+ continue;
+ /* skip peers with special export types */
+ if (peer->conf.export_type == EXPORT_NONE ||
+ peer->conf.export_type == EXPORT_DEFAULT_ROUTE)
+ continue;
+
up_generate_updates(out_rules, peer, new, old);
}
}
Index: rde_update.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
retrieving revision 1.123
diff -u -p -r1.123 rde_update.c
--- rde_update.c24 Jan 2020 05:44:05 - 1.123
+++ rde_update.c7 Jan 2021 18:13:45 -
@@ -47,11 +47,9 @@ static struct community comm_no_expsubco
static int
up_test_update(struct rde_peer *peer, struct prefix *p)
{
- struct bgpd_addr addr;
struct rde_aspath *asp;
struct rde_community*comm;
struct rde_peer *prefp;
- struct attr *attr;
if (p == NULL)
/* no prefix available */
@@ -70,10 +68,6 @@ up_test_update(struct rde_peer *peer, st
if (asp->flags & F_ATTR_LOOP)
fatalx("try to send out a looped path");
- pt_getaddr(p->pt, &addr);
- if (peer->capa.mp[addr.aid] == 0)
- return (-1);
-
if (!prefp->conf.ebgp && !peer->conf.ebgp) {
/*
* route reflector redistribution rules:
@@ -90,16 +84,6 @@ up_test_update(struct rde_peer *peer, st
return (0);
}
- /* export type handling */
- if (peer->conf.export_type == EXPORT_NONE ||
- peer->conf.export_type == EXPORT_DEFAULT_ROUTE) {
- /*
-* no need to withdraw old prefix as this will be
-* filtered out as well.
-*/
- return (-1);
- }
-
/* well known communities */
if (community_match(comm, &comm_no_advertise, NULL))
return (0);
@@ -110,18 +94,6 @@ up_test_update(struct rde_peer *peer, st
return (0);
}
- /*
-* Don't send messages back to originator
-* this is not specified in the RFC but seems logical.
-*/
- if ((attr = attr_optget(asp, ATTR_ORIGINATOR_ID)) != NULL) {
- if (memcmp(attr->data, &peer->remote_bgpid,
- sizeof(peer->remote_bgpid)) == 0) {
- /* would cause loop don't send */
- return (-1);
- }
- }
-
return (1);
}
@@ -149,13 +121,8 @@ withdraw:
peer->up_wcnt++;
}
} else {
- switch (up_test_update(peer, new)) {
- case 1:
-
Re: Port httpd(8) 'strip' directive to relayd(8)
On Thu, Jan 07, 2021 at 04:56:14PM +0100, Denis Fondras wrote:
> Le Thu, Jan 07, 2021 at 12:03:54PM +0100, Hiltjo Posthuma a écrit :
> > Hi Denis,
> >
> > I like this feature. For example it would be useful for using relayd as a
> > reverse-proxy to forward it to an internal network running a httpd with some
> > service. Then the path can be stripped without having to touch this service
> > configuration.
> >
> > Like: https://example.com/myservice/ -> http://192.168.0.2/ .
> >
> > I've noticed a small thing while testing the patch. When the path is "/" and
> > "strip 1" is used it becomes "", the request becomes: "GET HTTP/1.0". Maybe
> > this should be instead: "/". The same thing happens with a "strip number"
> > higher than the amount of sub paths.
> >
> > It could be worked-around by prefiltering with a match rule, but maybe it is
> > more obvious to make the root "/" ? The way the function
> > server_root_strip() is
> > used by OpenBSD httpd is that it first does a filesystem path check/open(2).
> >
> >
>
Hi Denis,
Awesome, tested the patch below and it looks good to me!
> Thank you for testing.
>
> Here is an update:
>
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
> retrieving revision 1.250
> diff -u -p -r1.250 parse.y
> --- parse.y 29 Dec 2020 19:48:06 - 1.250
> +++ parse.y 7 Jan 2021 15:08:28 -
> @@ -175,7 +175,7 @@ typedef struct {
> %token LOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON PARENT
> PATH
> %token PFTAG PORT PREFORK PRIORITY PROTO QUERYSTR REAL REDIRECT RELAY
> REMOVE
> %token REQUEST RESPONSE RETRY QUICK RETURN ROUNDROBIN ROUTE SACK
> SCRIPT SEND
> -%token SESSION SOCKET SPLICE SSL STICKYADDR STYLE TABLE TAG TAGGED TCP
> +%token SESSION SOCKET SPLICE SSL STICKYADDR STRIP STYLE TABLE TAG
> TAGGED TCP
> %token TIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT URL WITH TTL RTABLE
> %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE
> PASSWORD ECDHE
> %token EDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES
> CHECKS
> @@ -1549,6 +1549,20 @@ ruleopts : METHOD STRING
> {
> rule->rule_kv[keytype].kv_option = $2;
> rule->rule_kv[keytype].kv_type = keytype;
> }
> + | PATH STRIP NUMBER {
> + char*strip = NULL;
> +
> + if ($3 < 0 || $3 > INT_MAX) {
> + yyerror("invalid strip number");
> + YYERROR;
> + }
> + if (asprintf(&strip, "%lld", $3) <= 0)
> + fatal("can't parse strip");
> + keytype = KEY_TYPE_PATH;
> + rule->rule_kv[keytype].kv_option = KEY_OPTION_STRIP;
> + rule->rule_kv[keytype].kv_value = strip;
> + rule->rule_kv[keytype].kv_type = keytype;
> + }
> | QUERYSTR key_option STRING value {
> switch ($2) {
> case KEY_OPTION_APPEND:
> @@ -2481,6 +2495,7 @@ lookup(char *s)
> { "ssl",SSL },
> { "state", STATE },
> { "sticky-address", STICKYADDR },
> + { "strip", STRIP },
> { "style", STYLE },
> { "table", TABLE },
> { "tag",TAG },
> Index: relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.251
> diff -u -p -r1.251 relay.c
> --- relay.c 14 May 2020 17:27:38 - 1.251
> +++ relay.c 7 Jan 2021 15:08:28 -
> @@ -214,6 +214,9 @@ relay_ruledebug(struct relay_rule *rule)
> case KEY_OPTION_LOG:
> fprintf(stderr, "log ");
> break;
> + case KEY_OPTION_STRIP:
> + fprintf(stderr, "strip ");
> + break;
> case KEY_OPTION_NONE:
> break;
> }
> @@ -227,13 +230,15 @@ relay_ruledebug(struct relay_rule *rule)
> break;
> }
>
> + int kvv = (kv->kv_option == KEY_OPTION_STRIP ||
> + kv->kv_value == NULL);
> fprintf(stderr, "%s%s%s%s%s%s ",
> kv->kv_key == NULL ? "" : "\"",
> kv->kv_key == NULL ? "" : kv->kv_key,
> kv->kv_key == NULL ? "" : "\"",
> - kv->kv_value == NULL ? "" : " value \"",
> + kvv ? "" : " value \"",
> kv->kv_value == NULL ? "" : kv->kv_value,
> - kv->kv_value == NULL ? "" : "\"");
> +
Re: New ujoy(4) device for USB gamecontrollers
On Wed, Jan 06, 2021 at 10:48:58PM +0100, Marcus Glocker wrote: [...] > The implementation as such looks fine to me. > But I quickly gave the diff a spin before on amd64 using my PS > controller, and the result is not what I would expect. > > With uhid, I can access the controller on /dev/uhid6. The attach looks > like this: > > imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, > 4 ctls > imac /bsd: audio1 at uaudio0 > imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > imac /bsd: uhidev6: iclass 3/0, 180 report ids > imac /bsd: uhid6 at uhidev6 reportid 1: input=63, output=0, feature=0 [...] > imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, feature=63 > imac /bsd: uhid51 at uhidev6 reportid 180: input=0, output=0, feature=63 > > ujoy instead attached to previous uhid51, and there it is of no use > obviously. I still can access the controller through uhid6. The attach > with ujoy looks like this: > > imac /bsd: uaudio0 at uhub4 port 4 configuration 1 interface 1 "Sony > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > imac /bsd: uaudio0: class v1, full-speed, sync, channels: 2 play, 1 rec, > 4 ctls > imac /bsd: audio1 at uaudio0 > imac /bsd: uhidev6 at uhub4 port 4 configuration 1 interface 3 "Sony > Interactive Entertainment Wireless Controller" rev 2.00/1.00 addr 6 > imac /bsd: uhidev6: iclass 3/0, 180 report ids [...] > imac /bsd: uhid50 at uhidev6 reportid 179: input=0, output=0, feature=63 > imac /bsd: ujoy0 at uhidev6 reportid 180: input=0, output=0, feature=63 > > I haven't analyzed yet what happens in the code. > I can provide an lsusb of the controller if it's more obvious to you. I have heard from others who tried the diff that the PS4 controller is causing problems with the way it attaches. I ordered one to trial-and- error this myself at home. Could you share output of lsusb -vv? Thanks for giving it a try!
all platforms: isolate hardclock(9) from statclock()
Hi,
I want to isolate statclock() from hardclock(9). This will simplify
the logic in my WIP dynamic clock interrupt framework.
Currently, if stathz is zero, we call statclock() from within
hardclock(9). It looks like this (see sys/kern/kern_clock.c):
void
hardclock(struct clockframe *frame)
{
/* [...] */
if (stathz == 0)
statclock(frame);
/* [...] */
This is the case on alpha, amd64 (w/ lapic), hppa, i386 (w/ lapic),
loongson, luna88k, mips64, and sh.
(We seem to do it on sgi, too. I was under the impression that sgi
*was* a mips64 platform, yet sgi seems to it have its own clock
interrupt code distinct from mips64's general clock interrupt code
which is used by e.g. octeon).
However, if stathz is not zero we call statclock() separately. This
is the case on armv7, arm, arm64, macppc, powerpc64, and sparc64.
(The situation for the general powerpc code and socppc in particular
is a mystery to me.)
If we could remove this MD distinction it would make my MI framework
simpler. Instead of checking stathz and conditionally starting a
statclock event I will be able to unconditionally start a statclock
event on all platforms on every CPU.
In general I don't think the "is stathz zero?" variance between
platforms is useful:
- The difference is invisible to userspace, as we hide the fact that
stathz is zero from e.g. the kern.clockrate sysctl.
- We run statclock() at some point, regardless of whether stathz is
zero. If statclock() is run from hardclock(9) then isn't stathz
effectively equal to hz?
- Because stathz might be zero we need to add a bunch of safety checks
to our MI code to ensure we don't accidentally divide by zero.
Maybe we can ensure stathz is non-zero in a later diff...
--
Anyway, I don't think I have missed any platforms. However, if
platform experts could weigh in here to verify my changes (and test
them!) I'd really appreciate it.
In particular, I'm confused about how clock interrupts work on
powerpc, socppc, and sgi.
--
Thoughts? Platform-specific OKs?
Index: sys/kern/kern_clock.c
===
RCS file: /cvs/src/sys/kern/kern_clock.c,v
retrieving revision 1.101
diff -u -p -r1.101 kern_clock.c
--- sys/kern/kern_clock.c 21 Jan 2020 16:16:23 - 1.101
+++ sys/kern/kern_clock.c 7 Jan 2021 16:37:09 -
@@ -164,12 +164,6 @@ hardclock(struct clockframe *frame)
}
}
- /*
-* If no separate statistics clock is available, run it from here.
-*/
- if (stathz == 0)
- statclock(frame);
-
if (--ci->ci_schedstate.spc_rrticks <= 0)
roundrobin(ci);
Index: sys/arch/alpha/alpha/clock.c
===
RCS file: /cvs/src/sys/arch/alpha/alpha/clock.c,v
retrieving revision 1.24
diff -u -p -r1.24 clock.c
--- sys/arch/alpha/alpha/clock.c6 Jul 2020 13:33:06 - 1.24
+++ sys/arch/alpha/alpha/clock.c7 Jan 2021 16:37:09 -
@@ -136,6 +136,13 @@ clockattach(dev, fns)
* Machine-dependent clock routines.
*/
+void
+clockintr(struct clockframe *frame)
+{
+ hardclock(frame);
+ statclock(frame);
+}
+
/*
* Start the real-time and statistics clocks. Leave stathz 0 since there
* are no other timers available.
@@ -165,7 +172,7 @@ cpu_initclocks(void)
* hardclock, which would then fall over because the pointer
* to the virtual timers wasn't set at that time.
*/
- platform.clockintr = hardclock;
+ platform.clockintr = clockintr;
schedhz = 16;
evcount_attach(&clk_count, "clock", &clk_irq);
Index: sys/arch/amd64/amd64/lapic.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.57
diff -u -p -r1.57 lapic.c
--- sys/arch/amd64/amd64/lapic.c6 Sep 2020 20:50:00 - 1.57
+++ sys/arch/amd64/amd64/lapic.c7 Jan 2021 16:37:09 -
@@ -452,6 +452,7 @@ lapic_clockintr(void *arg, struct intrfr
floor = ci->ci_handled_intr_level;
ci->ci_handled_intr_level = ci->ci_ilevel;
hardclock((struct clockframe *)&frame);
+ statclock((struct clockframe *)&frame);
ci->ci_handled_intr_level = floor;
clk_count.ec_count++;
Index: sys/arch/hppa/dev/clock.c
===
RCS file: /cvs/src/sys/arch/hppa/dev/clock.c,v
retrieving revision 1.31
diff -u -p -r1.31 clock.c
--- sys/arch/hppa/dev/clock.c 6 Jul 2020 13:33:07 - 1.31
+++ sys/arch/hppa/dev/clock.c 7 Jan 2021 16:37:09 -
@@ -43,7 +43,7 @@
u_long cpu_hzticks;
-intcpu_hardclock(void *);
+intcpu_clockintr(void *);
u_int itmr_get_timecount(struct timecounter *);
struct timecounter itmr_timecounter = {
@@ -106,7 +106,7 @@ cpu_initclocks(void)
}
int
-cpu_hardclock(void *v)
+cpu_cl
Re: Port httpd(8) 'strip' directive to relayd(8)
Le Thu, Jan 07, 2021 at 12:03:54PM +0100, Hiltjo Posthuma a écrit :
> Hi Denis,
>
> I like this feature. For example it would be useful for using relayd as a
> reverse-proxy to forward it to an internal network running a httpd with some
> service. Then the path can be stripped without having to touch this service
> configuration.
>
> Like: https://example.com/myservice/ -> http://192.168.0.2/ .
>
> I've noticed a small thing while testing the patch. When the path is "/" and
> "strip 1" is used it becomes "", the request becomes: "GET HTTP/1.0". Maybe
> this should be instead: "/". The same thing happens with a "strip number"
> higher than the amount of sub paths.
>
> It could be worked-around by prefiltering with a match rule, but maybe it is
> more obvious to make the root "/" ? The way the function server_root_strip()
> is
> used by OpenBSD httpd is that it first does a filesystem path check/open(2).
>
>
Thank you for testing.
Here is an update:
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.250
diff -u -p -r1.250 parse.y
--- parse.y 29 Dec 2020 19:48:06 - 1.250
+++ parse.y 7 Jan 2021 15:08:28 -
@@ -175,7 +175,7 @@ typedef struct {
%token LOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON PARENT PATH
%token PFTAG PORT PREFORK PRIORITY PROTO QUERYSTR REAL REDIRECT RELAY REMOVE
%token REQUEST RESPONSE RETRY QUICK RETURN ROUNDROBIN ROUTE SACK SCRIPT SEND
-%token SESSION SOCKET SPLICE SSL STICKYADDR STYLE TABLE TAG TAGGED TCP
+%token SESSION SOCKET SPLICE SSL STICKYADDR STRIP STYLE TABLE TAG TAGGED TCP
%token TIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT URL WITH TTL RTABLE
%token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD ECDHE
%token EDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES CHECKS
@@ -1549,6 +1549,20 @@ ruleopts : METHOD STRING
{
rule->rule_kv[keytype].kv_option = $2;
rule->rule_kv[keytype].kv_type = keytype;
}
+ | PATH STRIP NUMBER {
+ char*strip = NULL;
+
+ if ($3 < 0 || $3 > INT_MAX) {
+ yyerror("invalid strip number");
+ YYERROR;
+ }
+ if (asprintf(&strip, "%lld", $3) <= 0)
+ fatal("can't parse strip");
+ keytype = KEY_TYPE_PATH;
+ rule->rule_kv[keytype].kv_option = KEY_OPTION_STRIP;
+ rule->rule_kv[keytype].kv_value = strip;
+ rule->rule_kv[keytype].kv_type = keytype;
+ }
| QUERYSTR key_option STRING value {
switch ($2) {
case KEY_OPTION_APPEND:
@@ -2481,6 +2495,7 @@ lookup(char *s)
{ "ssl",SSL },
{ "state", STATE },
{ "sticky-address", STICKYADDR },
+ { "strip", STRIP },
{ "style", STYLE },
{ "table", TABLE },
{ "tag",TAG },
Index: relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.251
diff -u -p -r1.251 relay.c
--- relay.c 14 May 2020 17:27:38 - 1.251
+++ relay.c 7 Jan 2021 15:08:28 -
@@ -214,6 +214,9 @@ relay_ruledebug(struct relay_rule *rule)
case KEY_OPTION_LOG:
fprintf(stderr, "log ");
break;
+ case KEY_OPTION_STRIP:
+ fprintf(stderr, "strip ");
+ break;
case KEY_OPTION_NONE:
break;
}
@@ -227,13 +230,15 @@ relay_ruledebug(struct relay_rule *rule)
break;
}
+ int kvv = (kv->kv_option == KEY_OPTION_STRIP ||
+kv->kv_value == NULL);
fprintf(stderr, "%s%s%s%s%s%s ",
kv->kv_key == NULL ? "" : "\"",
kv->kv_key == NULL ? "" : kv->kv_key,
kv->kv_key == NULL ? "" : "\"",
- kv->kv_value == NULL ? "" : " value \"",
+ kvv ? "" : " value \"",
kv->kv_value == NULL ? "" : kv->kv_value,
- kv->kv_value == NULL ? "" : "\"");
+ kvv ? "" : "\"");
}
if (rule->rule_tablename[0])
Index: relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.79
diff -u -p -r1.79 relay_http.c
--- relay_http.c4 Sep 2020 13:09
extend ip(4) to document ip_mreqn
Here is my try to extend ip(4) to also document struct ip_mreqn.
Not sure what is the best way to document the option to use either struct
ip_mreq or struct ip_mreqn with IP_ADD_MEMBERSHIP.
--
:wq Claudio
Index: ip.4
===
RCS file: /cvs/src/share/man/man4/ip.4,v
retrieving revision 1.41
diff -u -p -r1.41 ip.4
--- ip.418 Aug 2016 11:45:18 - 1.41
+++ ip.47 Jan 2021 14:58:54 -
@@ -411,13 +411,21 @@ setsockopt(s, IPPROTO_IP, IP_ADD_MEMBERS
.Pp
where
.Fa mreq
-is the following structure:
+is either the following structure:
.Bd -literal -offset indent
struct ip_mreq {
struct in_addr imr_multiaddr; /* multicast group to join */
struct in_addr imr_interface; /* interface to join on */
}
.Ed
+or
+.Bd -literal -offset indent
+struct ip_mreqn {
+struct in_addr imr_multiaddr; /* multicast group to join */
+struct in_addr imr_address; /* local IP address of interface */
+intimr_ifindex; /* interface index to join*/
+};
+.Ed
.Pp
.Va imr_interface
should
@@ -428,6 +436,12 @@ or the
.Tn IP
address of a particular multicast-capable interface if
the host is multihomed.
+.Va imr_ifindex
+of
+.Va struct ip_mreqn
+can be set to the interface index instead of specifying the
+.Tn IP
+address of a particular multicast-capable interface.
Membership is associated with a single interface;
programs running on multihomed hosts may need to
join the same group on more than one interface.
Re: rasops1
> Date: Thu, 7 Jan 2021 14:24:58 +0100
> From: Frederic Cambus
>
> On Thu, Dec 24, 2020 at 12:25:40AM +0100, Patrick Wildt wrote:
>
> > > On Fri, Dec 18, 2020 at 10:33:52PM +0100, Mark Kettenis wrote:
> > >
> > > > The diff below disables the optimized functions on little-endian
> > > > architectures such that we always use rasops1_putchar(). This makes
> > > > ssdfb(4) work with the default 8x16 font on arm64.
> > >
> > > I noticed it was committed already, but it seems the following
> > > directives:
> > >
> > > #if defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> > >
> > > Should have been:
> > >
> > > #if !defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> > >
> > > We want to include the optimized putchar functions only if RASOPS_SMALL
> > > is not defined.
> > >
> >
> > True that. In one #endif comment he actually kept the !, but the actual
> > ifs lost it.
>
> Here is a diff to fix the issue, which includes the optimized putchar
> functions only if RASOPS_SMALL is not defined.
>
> Comments? OK?
ah yes. Sorry, I forgot about that.
ok kettenis@
> Index: sys/dev/rasops/rasops1.c
> ===
> RCS file: /cvs/src/sys/dev/rasops/rasops1.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 rasops1.c
> --- sys/dev/rasops/rasops1.c 21 Dec 2020 12:58:42 - 1.11
> +++ sys/dev/rasops/rasops1.c 7 Jan 2021 11:08:55 -
> @@ -44,7 +44,7 @@ int rasops1_copycols(void *, int, int, i
> int rasops1_erasecols(void *, int, int, int, uint32_t);
> int rasops1_do_cursor(struct rasops_info *);
> int rasops1_putchar(void *, int, int col, u_int, uint32_t);
> -#if defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> +#if !defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> int rasops1_putchar8(void *, int, int col, u_int, uint32_t);
> int rasops1_putchar16(void *, int, int col, u_int, uint32_t);
> #endif
> @@ -58,7 +58,7 @@ rasops1_init(struct rasops_info *ri)
> rasops_masks_init();
>
> switch (ri->ri_font->fontwidth) {
> -#if defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> +#if !defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> case 8:
> ri->ri_ops.putchar = rasops1_putchar8;
> break;
> @@ -223,7 +223,7 @@ rasops1_putchar(void *cookie, int row, i
> return 0;
> }
>
> -#if defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> +#if !defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> /*
> * Paint a single character. This is for 8-pixel wide fonts.
> */
>
rpki-client simplify entity queue handling
Currently rpki-client keeps all pending work on a queue and only removes
it from the queue at once it got processed. The only bit that the parent
rpki-client process needs from the queue is the type when processing the
response. So instead of passing the id pass the type back from the parser.
With this the queue only holds entries that can't be processed right now
because the repository is not yet loaded. Additionally the handling of
responses becomes more decoupled.
All in all I think this simplifies the code a fair bit. What do others
think?
--
:wq Claudio
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.88
diff -u -p -r1.88 main.c
--- main.c 21 Dec 2020 11:35:55 - 1.88
+++ main.c 7 Jan 2021 13:22:02 -
@@ -88,6 +88,7 @@ structrepo {
size_t id; /* identifier (array index) */
};
+size_t entity_queue;
inttimeout = 60*60;
volatile sig_atomic_t killme;
void suicide(int sig);
@@ -105,7 +106,6 @@ static struct repotab {
* and parsed.
*/
struct entity {
- size_t id; /* unique identifier */
enum rtype type; /* type of entity (not RTYPE_EOF) */
char*uri; /* file or rsync:// URI */
int has_dgst; /* whether dgst is specified */
@@ -223,7 +223,6 @@ static void
entity_read_req(int fd, struct entity *ent)
{
- io_simple_read(fd, &ent->id, sizeof(size_t));
io_simple_read(fd, &ent->type, sizeof(enum rtype));
io_str_read(fd, &ent->uri);
io_simple_read(fd, &ent->has_dgst, sizeof(int));
@@ -244,7 +243,6 @@ entity_buffer_req(char **b, size_t *bsz,
const struct entity *ent)
{
- io_simple_buffer(b, bsz, bmax, &ent->id, sizeof(size_t));
io_simple_buffer(b, bsz, bmax, &ent->type, sizeof(enum rtype));
io_str_buffer(b, bsz, bmax, ent->uri);
io_simple_buffer(b, bsz, bmax, &ent->has_dgst, sizeof(int));
@@ -278,12 +276,14 @@ entity_write_req(int fd, const struct en
static void
entityq_flush(int fd, struct entityq *q, const struct repo *repo)
{
- struct entity *p;
+ struct entity *p, *np;
- TAILQ_FOREACH(p, q, entries) {
+ TAILQ_FOREACH_SAFE(p, q, entries, np) {
if (p->repo < 0 || repo->id != (size_t)p->repo)
continue;
entity_write_req(fd, p);
+ TAILQ_REMOVE(q, p, entries);
+ entity_free(p);
}
}
@@ -365,49 +365,18 @@ repo_filename(const struct repo *repo, c
}
/*
- * Read the next entity from the parser process, removing it from the
- * queue of pending requests in the process.
- * This always returns a valid entity.
- */
-static struct entity *
-entityq_next(int fd, struct entityq *q)
-{
- size_t id;
- struct entity *entp;
-
- io_simple_read(fd, &id, sizeof(size_t));
-
- TAILQ_FOREACH(entp, q, entries)
- if (entp->id == id)
- break;
-
- assert(entp != NULL);
- TAILQ_REMOVE(q, entp, entries);
- return entp;
-}
-
-static void
-entity_buffer_resp(char **b, size_t *bsz, size_t *bmax,
-const struct entity *ent)
-{
-
- io_simple_buffer(b, bsz, bmax, &ent->id, sizeof(size_t));
-}
-
-/*
* Add the heap-allocated file to the queue for processing.
*/
static void
entityq_add(int fd, struct entityq *q, char *file, enum rtype type,
const struct repo *rp, const unsigned char *dgst,
-const unsigned char *pkey, size_t pkeysz, char *descr, size_t *eid)
+const unsigned char *pkey, size_t pkeysz, char *descr)
{
struct entity *p;
if ((p = calloc(1, sizeof(struct entity))) == NULL)
err(1, "calloc");
- p->id = (*eid)++;
p->type = type;
p->uri = file;
p->repo = (rp != NULL) ? (ssize_t)rp->id : -1;
@@ -426,15 +395,19 @@ entityq_add(int fd, struct entityq *q, c
err(1, "strdup");
filepath_add(file);
- TAILQ_INSERT_TAIL(q, p, entries);
+
+ entity_queue++;
/*
* Write to the queue if there's no repo or the repo has already
-* been loaded.
+* been loaded else enqueue it for later.
*/
- if (rp == NULL || rp->loaded)
+ if (rp == NULL || rp->loaded) {
entity_write_req(fd, p);
+ entity_free(p);
+ } else
+ TAILQ_INSERT_TAIL(q, p, entries);
}
/*
@@ -443,7 +416,7 @@ entityq_add(int fd, struct entityq *q, c
*/
static void
queue_add_from_mft(int fd, struct entityq *q, const char *mft,
-const struct mftfile *file, enum rtype type, size_t *eid)
+const struct mftfile *file, enum rtype type)
{
char*cp, *nfile;
@@ -461,7 +434,7 @@ queue_add_from_mft(int fd, struct entity
* that the repository has already been loaded.
*/
- entityq_add(fd, q, n
Re: rasops1
On Thu, Dec 24, 2020 at 12:25:40AM +0100, Patrick Wildt wrote:
> > On Fri, Dec 18, 2020 at 10:33:52PM +0100, Mark Kettenis wrote:
> >
> > > The diff below disables the optimized functions on little-endian
> > > architectures such that we always use rasops1_putchar(). This makes
> > > ssdfb(4) work with the default 8x16 font on arm64.
> >
> > I noticed it was committed already, but it seems the following
> > directives:
> >
> > #if defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> >
> > Should have been:
> >
> > #if !defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
> >
> > We want to include the optimized putchar functions only if RASOPS_SMALL
> > is not defined.
> >
>
> True that. In one #endif comment he actually kept the !, but the actual
> ifs lost it.
Here is a diff to fix the issue, which includes the optimized putchar
functions only if RASOPS_SMALL is not defined.
Comments? OK?
Index: sys/dev/rasops/rasops1.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops1.c,v
retrieving revision 1.11
diff -u -p -r1.11 rasops1.c
--- sys/dev/rasops/rasops1.c21 Dec 2020 12:58:42 - 1.11
+++ sys/dev/rasops/rasops1.c7 Jan 2021 11:08:55 -
@@ -44,7 +44,7 @@ int rasops1_copycols(void *, int, int, i
intrasops1_erasecols(void *, int, int, int, uint32_t);
intrasops1_do_cursor(struct rasops_info *);
intrasops1_putchar(void *, int, int col, u_int, uint32_t);
-#if defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
+#if !defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
intrasops1_putchar8(void *, int, int col, u_int, uint32_t);
intrasops1_putchar16(void *, int, int col, u_int, uint32_t);
#endif
@@ -58,7 +58,7 @@ rasops1_init(struct rasops_info *ri)
rasops_masks_init();
switch (ri->ri_font->fontwidth) {
-#if defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
+#if !defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
case 8:
ri->ri_ops.putchar = rasops1_putchar8;
break;
@@ -223,7 +223,7 @@ rasops1_putchar(void *cookie, int row, i
return 0;
}
-#if defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
+#if !defined(RASOPS_SMALL) && BYTE_ORDER == BIG_ENDIAN
/*
* Paint a single character. This is for 8-pixel wide fonts.
*/
Re: Extend IP_ADD_MEMBERSHIP to support struct ip_mreqn
On Wed, Jan 06, 2021 at 10:27:42AM +0100, Claudio Jeker wrote:
> Linux and FreeBSD both support the use of struct ip_mreqn in
> IP_ADD_MEMBERSHIP and IP_DROP_MEMBERSHIP. This struct adds one more field
> to pass an interface index to the kernel (instead of using the IP
> address).
>
> struct ip_mreqn {
>struct in_addr imr_multiaddr; /* IP multicast address of group */
>struct in_addr imr_address;/* local IP address of interface */
>int imr_ifindex;/* interface index */
> };
>
> So if imr_ifindex is not 0 then this value is used to define the outgoing
> interface instead of doing a lookup with imr_address.
> This is something I want to use in ospfd(8) to support unnumbered
> interfaces (or actually point-to-point interfaces using the same source
> IP).
>
This diff is better. I removed the change to in_addmulti() from the
previous version. There is no benefit of passing the interface index to
in_addmulti (instead of the ifp). in_addmulti() needs the ifp (not just
only the index) and it just adds a lot of unneccessary change to the diff.
--
:wq Claudio
Index: netinet/in.h
===
RCS file: /cvs/src/sys/netinet/in.h,v
retrieving revision 1.138
diff -u -p -r1.138 in.h
--- netinet/in.h22 Aug 2020 17:55:30 - 1.138
+++ netinet/in.h6 Jan 2021 08:31:46 -
@@ -360,6 +360,12 @@ struct ip_mreq {
struct in_addr imr_interface; /* local IP address of interface */
};
+struct ip_mreqn {
+ struct in_addr imr_multiaddr; /* IP multicast address of group */
+ struct in_addr imr_address;/* local IP address of interface */
+ int imr_ifindex;/* interface index */
+};
+
/*
* Argument for IP_PORTRANGE:
* - which range to search when port is unspecified at bind() or connect()
Index: netinet/ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.358
diff -u -p -r1.358 ip_output.c
--- netinet/ip_output.c 20 Dec 2020 21:15:47 - 1.358
+++ netinet/ip_output.c 7 Jan 2021 11:14:37 -
@@ -73,6 +73,7 @@
#endif /* IPSEC */
int ip_pcbopts(struct mbuf **, struct mbuf *);
+int ip_multicast_if(struct ip_mreqn *, u_int, unsigned int *);
int ip_setmoptions(int, struct ip_moptions **, struct mbuf *, u_int);
void ip_mloopback(struct ifnet *, struct mbuf *, struct sockaddr_in *);
static __inline u_int16_t __attribute__((__unused__))
@@ -1337,6 +1338,51 @@ ip_pcbopts(struct mbuf **pcbopt, struct
}
/*
+ * Lookup the interface based on the information in the ip_mreqn struct.
+ */
+int
+ip_multicast_if(struct ip_mreqn *mreq, u_int rtableid, unsigned int *ifidx)
+{
+ struct sockaddr_in sin;
+ struct rtentry *rt;
+
+ /*
+* In case userland provides the imr_ifindex use this as interface.
+* If no interface address was provided, use the interface of
+* the route to the given multicast address.
+*/
+ if (mreq->imr_ifindex != 0) {
+ *ifidx = mreq->imr_ifindex;
+ } else if (mreq->imr_address.s_addr == INADDR_ANY) {
+ memset(&sin, 0, sizeof(sin));
+ sin.sin_len = sizeof(sin);
+ sin.sin_family = AF_INET;
+ sin.sin_addr = mreq->imr_multiaddr;
+ rt = rtalloc(sintosa(&sin), RT_RESOLVE, rtableid);
+ if (!rtisvalid(rt)) {
+ rtfree(rt);
+ return EADDRNOTAVAIL;
+ }
+ *ifidx = rt->rt_ifidx;
+ rtfree(rt);
+ } else {
+ memset(&sin, 0, sizeof(sin));
+ sin.sin_len = sizeof(sin);
+ sin.sin_family = AF_INET;
+ sin.sin_addr = mreq->imr_address;
+ rt = rtalloc(sintosa(&sin), 0, rtableid);
+ if (!rtisvalid(rt) || !ISSET(rt->rt_flags, RTF_LOCAL)) {
+ rtfree(rt);
+ return EADDRNOTAVAIL;
+ }
+ *ifidx = rt->rt_ifidx;
+ rtfree(rt);
+ }
+
+ return 0;
+}
+
+/*
* Set the IP multicast options in response to user setsockopt().
*/
int
@@ -1345,12 +1391,12 @@ ip_setmoptions(int optname, struct ip_mo
{
struct in_addr addr;
struct in_ifaddr *ia;
- struct ip_mreq *mreq;
+ struct ip_mreqn mreqn;
struct ifnet *ifp = NULL;
struct ip_moptions *imo = *imop;
struct in_multi **immp;
- struct rtentry *rt;
struct sockaddr_in sin;
+ unsigned int ifidx;
int i, error = 0;
u_char loop;
@@ -1438,63 +1484,41 @@ ip_setmoptions(int optname, struct ip_mo
* Add a multicast group membership.
* Group must be a valid IP multicast address.
*/
- if (m == NULL || m->m_len != sizeof(struct ip_mreq)) {
+ if (m == N
Re: Port httpd(8) 'strip' directive to relayd(8)
On Sun, Jan 03, 2021 at 11:40:42AM +0100, Denis Fondras wrote:
> Le Fri, Dec 11, 2020 at 10:53:56AM +, Olivier Cherrier a écrit :
> >
> > Hello tech@,
> >
> > Is there any interest for this feature to be commited?
> > I find it very useful. Thank you Denis!
> >
>
> Here is an up to date diff, looking for OKs.
>
Hi Denis,
I like this feature. For example it would be useful for using relayd as a
reverse-proxy to forward it to an internal network running a httpd with some
service. Then the path can be stripped without having to touch this service
configuration.
Like: https://example.com/myservice/ -> http://192.168.0.2/ .
I've noticed a small thing while testing the patch. When the path is "/" and
"strip 1" is used it becomes "", the request becomes: "GET HTTP/1.0". Maybe
this should be instead: "/". The same thing happens with a "strip number"
higher than the amount of sub paths.
It could be worked-around by prefiltering with a match rule, but maybe it is
more obvious to make the root "/" ? The way the function server_root_strip() is
used by OpenBSD httpd is that it first does a filesystem path check/open(2).
My test config:
Command:
printf 'GET / HTTP/1.0\r\nConnection: Close\r\nHost: 127.0.0.1\r\n\r\n'
| \
nc -v 127.0.0.1 80
relayd.conf:
table { 127.0.0.1 }
table { 127.0.0.1 }
http protocol "t" {
tcp { nodelay }
return error
match tag "notag"
match path strip 1 tag "ok"
block tagged "notag"
}
relay "r" {
listen on "127.0.0.1" port 80
protocol "t"
forward to port 8080
}
Netcat (fake server):
nc -k -v -l 127.0.0.1 8080
The incoming request is then:
Connection received on localhost 45510
GET HTTP/1.0
Connection: Close
Host: 127.0.0.1
I made no changes to the diff below.
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
> retrieving revision 1.250
> diff -u -p -r1.250 parse.y
> --- parse.y 29 Dec 2020 19:48:06 - 1.250
> +++ parse.y 3 Jan 2021 10:38:26 -
> @@ -175,7 +175,7 @@ typedef struct {
> %token LOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON PARENT
> PATH
> %token PFTAG PORT PREFORK PRIORITY PROTO QUERYSTR REAL REDIRECT RELAY
> REMOVE
> %token REQUEST RESPONSE RETRY QUICK RETURN ROUNDROBIN ROUTE SACK
> SCRIPT SEND
> -%token SESSION SOCKET SPLICE SSL STICKYADDR STYLE TABLE TAG TAGGED TCP
> +%token SESSION SOCKET SPLICE SSL STICKYADDR STRIP STYLE TABLE TAG
> TAGGED TCP
> %token TIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT URL WITH TTL RTABLE
> %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE
> PASSWORD ECDHE
> %token EDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES
> CHECKS
> @@ -1549,6 +1549,20 @@ ruleopts : METHOD STRING
> {
> rule->rule_kv[keytype].kv_option = $2;
> rule->rule_kv[keytype].kv_type = keytype;
> }
> + | PATH STRIP NUMBER {
> + char*strip = NULL;
> +
> + if ($3 < 0 || $3 > INT_MAX) {
> + yyerror("invalid strip number");
> + YYERROR;
> + }
> + if (asprintf(&strip, "%lld", $3) <= 0)
> + fatal("can't parse strip");
> + keytype = KEY_TYPE_PATH;
> + rule->rule_kv[keytype].kv_option = KEY_OPTION_STRIP;
> + rule->rule_kv[keytype].kv_value = strip;
> + rule->rule_kv[keytype].kv_type = keytype;
> + }
> | QUERYSTR key_option STRING value {
> switch ($2) {
> case KEY_OPTION_APPEND:
> @@ -2481,6 +2495,7 @@ lookup(char *s)
> { "ssl",SSL },
> { "state", STATE },
> { "sticky-address", STICKYADDR },
> + { "strip", STRIP },
> { "style", STYLE },
> { "table", TABLE },
> { "tag",TAG },
> Index: relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.251
> diff -u -p -r1.251 relay.c
> --- relay.c 14 May 2020 17:27:38 - 1.251
> +++ relay.c 3 Jan 2021 10:38:27 -
> @@ -214,6 +214,9 @@ relay_ruledebug(struct relay_rule *rule)
> case KEY_OPTION_LOG:
> fprintf(stderr, "log ");
> break;
> + case KEY_OPTION_STRIP:
> + fprintf(stderr, "strip ");
> + break;
> case KEY_OPTION_NONE:
> break;
>
Re: Extend IP_ADD_MEMBERSHIP to support struct ip_mreqn
On Wed, Jan 06, 2021 at 10:27:42AM +0100, Claudio Jeker wrote:
> Linux and FreeBSD both support the use of struct ip_mreqn in
> IP_ADD_MEMBERSHIP and IP_DROP_MEMBERSHIP. This struct adds one more field
> to pass an interface index to the kernel (instead of using the IP
> address).
>
> struct ip_mreqn {
>struct in_addr imr_multiaddr; /* IP multicast address of group */
>struct in_addr imr_address;/* local IP address of interface */
>int imr_ifindex;/* interface index */
> };
>
> So if imr_ifindex is not 0 then this value is used to define the outgoing
> interface instead of doing a lookup with imr_address.
> This is something I want to use in ospfd(8) to support unnumbered
> interfaces (or actually point-to-point interfaces using the same source
> IP).
>
Here the corresponding ospfd(8) diff to use struct ip_mreqn and the
interface index. With this it should be possible the have the same IP
address set on multiple interfaces.
--
:wq Claudio
Index: interface.c
===
RCS file: /cvs/src/usr.sbin/ospfd/interface.c,v
retrieving revision 1.84
diff -u -p -r1.84 interface.c
--- interface.c 2 Nov 2020 00:30:56 - 1.84
+++ interface.c 6 Jan 2021 09:33:38 -
@@ -711,7 +711,7 @@ LIST_HEAD(,if_group_count) ifglist = LIS
int
if_join_group(struct iface *iface, struct in_addr *addr)
{
- struct ip_mreq mreq;
+ struct ip_mreqn mreq;
struct if_group_count *ifg;
switch (iface->type) {
@@ -734,7 +734,7 @@ if_join_group(struct iface *iface, struc
return (0);
mreq.imr_multiaddr.s_addr = addr->s_addr;
- mreq.imr_interface.s_addr = iface->addr.s_addr;
+ mreq.imr_ifindex = iface->ifindex;
if (setsockopt(iface->fd, IPPROTO_IP, IP_ADD_MEMBERSHIP,
(void *)&mreq, sizeof(mreq)) == -1) {
@@ -760,7 +760,7 @@ if_join_group(struct iface *iface, struc
int
if_leave_group(struct iface *iface, struct in_addr *addr)
{
- struct ip_mreq mreq;
+ struct ip_mreqn mreq;
struct if_group_count *ifg;
switch (iface->type) {
@@ -782,7 +782,7 @@ if_leave_group(struct iface *iface, stru
}
mreq.imr_multiaddr.s_addr = addr->s_addr;
- mreq.imr_interface.s_addr = iface->addr.s_addr;
+ mreq.imr_ifindex = iface->ifindex;
if (setsockopt(iface->fd, IPPROTO_IP, IP_DROP_MEMBERSHIP,
(void *)&mreq, sizeof(mreq)) == -1) {
