Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

On Wed, May 24, 2023 at 11:43:03PM +0200, Hrvoje Popovski wrote:
> On 23.5.2023. 21:13, Klemens Nanni wrote:
> > On Sat, Jan 14, 2023 at 02:28:27PM +, Stuart Henderson wrote:
> >> On 2023/01/12 04:49, Mikolaj Kucharski wrote:
> >>> Hi,
> >>>
> >>> Is there anything else which I can do, to help this diff reviwed and
> >>> increase the chance of getting in?
> >>>
> >>> Thread at https://marc.info/?t=16347829861=1=2
> >>>
> >>> Last version of the diff at
> >>> https://marc.info/?l=openbsd-tech=167185582521873=mbox
> >> Inlining that for a few comments, otherwise it's ok sthen
> > wgdescr[iption] would be consistent with the existing descr[iption].
> > At least my keep typing the trailing "r"...
> > 
> > Then '-wgdescr' and 'wgdescr ""' work and are implemented exactly like
> > te inteface description equivalents.
> > 
> > I could use this now in a new VPN setup, so here's a polished diff,
> > with the above, missing ifconfig.8 bits written and other nits inline.
> > 
> > As Theo suggested, I'd drop the wg.4 and leave it to ifconfig.8 proper.
> > 
> > Feedback?
> > 
> > Either way, net/wireguard-tools needs a bump/rebuild.
> > 
> 
> Hi,
> 
> this would be nice features when having wg server with lots of wgpeers.
> 
> I've tried this diff ans it's working as expected.
> 
> 
> Thank you Mikolaj and kn@
> 

I just rebased the diff. Thanks Klemens for pushing this forward.

I would like to point out, that initial version was written by

Noah Meier 

https://marc.info/?l=openbsd-tech=163478285129091=2

-- 
Regards,
 Mikolaj

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

Hi,

Is there anything else which I can do, to help this diff reviwed and
increase the chance of getting in?

Thread at https://marc.info/?t=16347829861=1=2

Last version of the diff at
https://marc.info/?l=openbsd-tech=167185582521873=mbox


On Thu, Jan 05, 2023 at 07:41:54PM +0100, Hrvoje Popovski wrote:
> On 2.1.2023. 22:01, Mikolaj Kucharski wrote:
> > This seems to work fine for me.
> > 
> > Patch also available at:
> > 
> > https://marc.info/?l=openbsd-tech=167185582521873=mbox
> > 
> 
> I've had some problems with 20+ wgpeers few days ago and at that time it
> would have been good if I had wgdesc in ifconfig wg output ...
> 
> 
> > 
> > On Sat, Dec 24, 2022 at 03:29:35AM +, Mikolaj Kucharski wrote:
> >> On Sat, Nov 19, 2022 at 12:03:59PM +, Mikolaj Kucharski wrote:
> >>> Kind reminder.
> >>>
> >>> Below diff also at:
> >>>
> >>> https://marc.info/?l=openbsd-tech=166806412910623=2
> >>>
> >>> This is diff by Noah Meier with small changes by me.
> >>>
> >>>
> >>> On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
> >>>> On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> >>>>> On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> While wireguard interfaces can have a description set by ifconfig, 
> >>>>>> wireguard peers currently cannot. I now have a lot of peers and 
> >>>>>> descriptions of them in ifconfig would be helpful.
> >>>>>>
> >>>>>> This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a 
> >>>>>> corresponding '-wgdesc' option). Man page also updated.
> >>>>>>
> >>>>>> NM
> >>>>>
> >>>>> Now that my `ifconfig, wireguard output less verbose, unless -A or `
> >>>>> diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> >>>>> bump of an old thread.
> >>>>>
> >>>>> Below is rebased on -current and tiny modified by me, Noah's diff.
> >>>>>
> >>>>> You need both kernel and ifconfig with below code, otherwise you may see
> >>>>> issues bringing up wg(4) interface. If you may loose access to machine
> >>>>> behind wg(4) VPN, make sure you update on that machine both kernel and
> >>>>> ifconfig(8) at the same time.
> >>>>>
> >>
> >> Rebased again, just a moment ago. Will test runtime again over the weekend,
> >> are there no surprises.
> >>
> >> - ifconfig compiles
> >> - GENERIC.MP/amd64 kernel compiles too
> >>
> >>
> >> Index: sbin/ifconfig/ifconfig.c
> >> ===
> >> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> >> retrieving revision 1.460
> >> diff -u -p -u -r1.460 ifconfig.c
> >> --- sbin/ifconfig/ifconfig.c   18 Dec 2022 18:56:38 -  1.460
> >> +++ sbin/ifconfig/ifconfig.c   24 Dec 2022 00:49:05 -
> >> @@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
> >>  void  setwgpeeraip(const char *, int);
> >>  void  setwgpeerpsk(const char *, int);
> >>  void  setwgpeerpka(const char *, int);
> >> +void  setwgpeerdesc(const char *, int);
> >>  void  setwgport(const char *, int);
> >>  void  setwgkey(const char *, int);
> >>  void  setwgrtable(const char *, int);
> >>  
> >>  void  unsetwgpeer(const char *, int);
> >>  void  unsetwgpeerpsk(const char *, int);
> >> +void  unsetwgpeerdesc(const char *, int);
> >>  void  unsetwgpeerall(const char *, int);
> >>  
> >>  void  wg_status(int);
> >> @@ -623,11 +625,13 @@ const struct cmd {
> >>{ "wgaip",  NEXTARG,A_WIREGUARD,setwgpeeraip},
> >>{ "wgpsk",  NEXTARG,A_WIREGUARD,setwgpeerpsk},
> >>{ "wgpka",  NEXTARG,A_WIREGUARD,setwgpeerpka},
> >> +  { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
> >>{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
> >>{ "wgkey",  NEXTARG,A_WIREGUARD,setwgkey},
> >>{ "wgrtable",   NEXTARG,A_WIREGUARD,   

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

This seems to work fine for me.

Patch also available at:

https://marc.info/?l=openbsd-tech=167185582521873=mbox


On Sat, Dec 24, 2022 at 03:29:35AM +, Mikolaj Kucharski wrote:
> On Sat, Nov 19, 2022 at 12:03:59PM +0000, Mikolaj Kucharski wrote:
> > Kind reminder.
> > 
> > Below diff also at:
> > 
> > https://marc.info/?l=openbsd-tech=166806412910623=2
> > 
> > This is diff by Noah Meier with small changes by me.
> > 
> > 
> > On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
> > > On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> > > > On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > > > > Hi,
> > > > > 
> > > > > While wireguard interfaces can have a description set by ifconfig, 
> > > > > wireguard peers currently cannot. I now have a lot of peers and 
> > > > > descriptions of them in ifconfig would be helpful.
> > > > > 
> > > > > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a 
> > > > > corresponding '-wgdesc' option). Man page also updated.
> > > > > 
> > > > > NM
> > > > 
> > > > Now that my `ifconfig, wireguard output less verbose, unless -A or `
> > > > diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> > > > bump of an old thread.
> > > > 
> > > > Below is rebased on -current and tiny modified by me, Noah's diff.
> > > > 
> > > > You need both kernel and ifconfig with below code, otherwise you may see
> > > > issues bringing up wg(4) interface. If you may loose access to machine
> > > > behind wg(4) VPN, make sure you update on that machine both kernel and
> > > > ifconfig(8) at the same time.
> > > > 
> 
> Rebased again, just a moment ago. Will test runtime again over the weekend,
> are there no surprises.
> 
> - ifconfig compiles
> - GENERIC.MP/amd64 kernel compiles too
> 
> 
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.460
> diff -u -p -u -r1.460 ifconfig.c
> --- sbin/ifconfig/ifconfig.c  18 Dec 2022 18:56:38 -  1.460
> +++ sbin/ifconfig/ifconfig.c  24 Dec 2022 00:49:05 -
> @@ -355,12 +355,14 @@ voidsetwgpeerep(const char *, const cha
>  void setwgpeeraip(const char *, int);
>  void setwgpeerpsk(const char *, int);
>  void setwgpeerpka(const char *, int);
> +void setwgpeerdesc(const char *, int);
>  void setwgport(const char *, int);
>  void setwgkey(const char *, int);
>  void setwgrtable(const char *, int);
>  
>  void unsetwgpeer(const char *, int);
>  void unsetwgpeerpsk(const char *, int);
> +void unsetwgpeerdesc(const char *, int);
>  void unsetwgpeerall(const char *, int);
>  
>  void wg_status(int);
> @@ -623,11 +625,13 @@ const structcmd {
>   { "wgaip",  NEXTARG,A_WIREGUARD,setwgpeeraip},
>   { "wgpsk",  NEXTARG,A_WIREGUARD,setwgpeerpsk},
>   { "wgpka",  NEXTARG,A_WIREGUARD,setwgpeerpka},
> + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
>   { "wgport", NEXTARG,A_WIREGUARD,setwgport},
>   { "wgkey",  NEXTARG,A_WIREGUARD,setwgkey},
>   { "wgrtable",   NEXTARG,A_WIREGUARD,setwgrtable},
>   { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
>   { "-wgpsk", 0,  A_WIREGUARD,unsetwgpeerpsk},
> + { "-wgdesc",0,  A_WIREGUARD,unsetwgpeerdesc},
>   { "-wgpeerall", 0,  A_WIREGUARD,unsetwgpeerall},
>  
>  #else /* SMALL */
> @@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
>  }
>  
>  void
> +setwgpeerdesc(const char *wgdesc, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + if (strlen(wgdesc))
> + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
> + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> +}
> +
> +void
>  setwgport(const char *port, int param)
>  {
>   const char *errmsg = NULL;
> @@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
>  }
>  
>  void
> +unsetwgpeerdesc(const char *value, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + str

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

On Sat, Nov 19, 2022 at 12:03:59PM +, Mikolaj Kucharski wrote:
> Kind reminder.
> 
> Below diff also at:
> 
> https://marc.info/?l=openbsd-tech=166806412910623=2
> 
> This is diff by Noah Meier with small changes by me.
> 
> 
> On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
> > On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> > > On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > > > Hi,
> > > > 
> > > > While wireguard interfaces can have a description set by ifconfig, 
> > > > wireguard peers currently cannot. I now have a lot of peers and 
> > > > descriptions of them in ifconfig would be helpful.
> > > > 
> > > > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a 
> > > > corresponding '-wgdesc' option). Man page also updated.
> > > > 
> > > > NM
> > > 
> > > Now that my `ifconfig, wireguard output less verbose, unless -A or `
> > > diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> > > bump of an old thread.
> > > 
> > > Below is rebased on -current and tiny modified by me, Noah's diff.
> > > 
> > > You need both kernel and ifconfig with below code, otherwise you may see
> > > issues bringing up wg(4) interface. If you may loose access to machine
> > > behind wg(4) VPN, make sure you update on that machine both kernel and
> > > ifconfig(8) at the same time.
> > > 

Rebased again, just a moment ago. Will test runtime again over the weekend,
are there no surprises.

- ifconfig compiles
- GENERIC.MP/amd64 kernel compiles too


Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.460
diff -u -p -u -r1.460 ifconfig.c
--- sbin/ifconfig/ifconfig.c18 Dec 2022 18:56:38 -  1.460
+++ sbin/ifconfig/ifconfig.c24 Dec 2022 00:49:05 -
@@ -355,12 +355,14 @@ void  setwgpeerep(const char *, const cha
 void   setwgpeeraip(const char *, int);
 void   setwgpeerpsk(const char *, int);
 void   setwgpeerpka(const char *, int);
+void   setwgpeerdesc(const char *, int);
 void   setwgport(const char *, int);
 void   setwgkey(const char *, int);
 void   setwgrtable(const char *, int);
 
 void   unsetwgpeer(const char *, int);
 void   unsetwgpeerpsk(const char *, int);
+void   unsetwgpeerdesc(const char *, int);
 void   unsetwgpeerall(const char *, int);
 
 void   wg_status(int);
@@ -623,11 +625,13 @@ const struct  cmd {
{ "wgaip",  NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk",  NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka",  NEXTARG,A_WIREGUARD,setwgpeerpka},
+   { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey",  NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable",   NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0,  A_WIREGUARD,unsetwgpeerpsk},
+   { "-wgdesc",0,  A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0,  A_WIREGUARD,unsetwgpeerall},
 
 #else /* SMALL */
@@ -5856,6 +5860,16 @@ setwgpeerpka(const char *pka, int param)
 }
 
 void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+   if (wg_peer == NULL)
+   errx(1, "wgdesc: wgpeer not set");
+   if (strlen(wgdesc))
+   strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+   wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
 setwgport(const char *port, int param)
 {
const char *errmsg = NULL;
@@ -5902,6 +5916,15 @@ unsetwgpeerpsk(const char *value, int pa
 }
 
 void
+unsetwgpeerdesc(const char *value, int param)
+{
+   if (wg_peer == NULL)
+   errx(1, "wgdesc: wgpeer not set");
+   strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+   wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
 unsetwgpeerall(const char *value, int param)
 {
ensurewginterface();
@@ -5961,6 +5984,9 @@ wg_status(int ifaliases)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+   if (strlen(wg_peer->p_description))
+   printf("\t\twgdesc %s\n", 
wg_peer->p_description);
 
if (wg_peer->p_flags & WG_PE

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

Kind reminder.

Below diff also at:

https://marc.info/?l=openbsd-tech=166806412910623=2

This is diff by Noah Meier with small changes by me.


On Thu, Nov 10, 2022 at 07:14:11AM +, Mikolaj Kucharski wrote:
> On Thu, Nov 10, 2022 at 12:53:07AM +0000, Mikolaj Kucharski wrote:
> > On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > > Hi,
> > > 
> > > While wireguard interfaces can have a description set by ifconfig, 
> > > wireguard peers currently cannot. I now have a lot of peers and 
> > > descriptions of them in ifconfig would be helpful.
> > > 
> > > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a 
> > > corresponding '-wgdesc' option). Man page also updated.
> > > 
> > > NM
> > 
> > Now that my `ifconfig, wireguard output less verbose, unless -A or `
> > diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> > bump of an old thread.
> > 
> > Below is rebased on -current and tiny modified by me, Noah's diff.
> > 
> > You need both kernel and ifconfig with below code, otherwise you may see
> > issues bringing up wg(4) interface. If you may loose access to machine
> > behind wg(4) VPN, make sure you update on that machine both kernel and
> > ifconfig(8) at the same time.
> > 
> 
> Typo, s/wgpesc/wgdesc/
> 
> 
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.457
> diff -u -p -u -r1.457 ifconfig.c
> --- sbin/ifconfig/ifconfig.c  26 Oct 2022 17:06:31 -  1.457
> +++ sbin/ifconfig/ifconfig.c  10 Nov 2022 01:03:04 -
> @@ -355,12 +355,14 @@ voidsetwgpeerep(const char *, const cha
>  void setwgpeeraip(const char *, int);
>  void setwgpeerpsk(const char *, int);
>  void setwgpeerpka(const char *, int);
> +void setwgpeerdesc(const char *, int);
>  void setwgport(const char *, int);
>  void setwgkey(const char *, int);
>  void setwgrtable(const char *, int);
>  
>  void unsetwgpeer(const char *, int);
>  void unsetwgpeerpsk(const char *, int);
> +void unsetwgpeerdesc(const char *, int);
>  void unsetwgpeerall(const char *, int);
>  
>  void wg_status(int);
> @@ -620,11 +622,13 @@ const structcmd {
>   { "wgaip",  NEXTARG,A_WIREGUARD,setwgpeeraip},
>   { "wgpsk",  NEXTARG,A_WIREGUARD,setwgpeerpsk},
>   { "wgpka",  NEXTARG,A_WIREGUARD,setwgpeerpka},
> + { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
>   { "wgport", NEXTARG,A_WIREGUARD,setwgport},
>   { "wgkey",  NEXTARG,A_WIREGUARD,setwgkey},
>   { "wgrtable",   NEXTARG,A_WIREGUARD,setwgrtable},
>   { "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
>   { "-wgpsk", 0,  A_WIREGUARD,unsetwgpeerpsk},
> + { "-wgdesc",0,  A_WIREGUARD,unsetwgpeerdesc},
>   { "-wgpeerall", 0,  A_WIREGUARD,unsetwgpeerall},
>  
>  #else /* SMALL */
> @@ -5843,6 +5847,16 @@ setwgpeerpka(const char *pka, int param)
>  }
>  
>  void
> +setwgpeerdesc(const char *wgdesc, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + if (strlen(wgdesc))
> + strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
> + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> +}
> +
> +void
>  setwgport(const char *port, int param)
>  {
>   const char *errmsg = NULL;
> @@ -5889,6 +5903,15 @@ unsetwgpeerpsk(const char *value, int pa
>  }
>  
>  void
> +unsetwgpeerdesc(const char *value, int param)
> +{
> + if (wg_peer == NULL)
> + errx(1, "wgdesc: wgpeer not set");
> + strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
> + wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
> +}
> +
> +void
>  unsetwgpeerall(const char *value, int param)
>  {
>   ensurewginterface();
> @@ -5948,6 +5971,9 @@ wg_status(int ifaliases)
>   b64_ntop(wg_peer->p_public, WG_KEY_LEN,
>   key, sizeof(key));
>   printf("\twgpeer %s\n", key);
> +
> + if (strlen(wg_peer->p_description))
> + printf("\t\twgdesc %s\n", 
> wg_peer->p_description);
>  
>   if (wg_peer->p_flags & WG_PEER_HAS_PSK)
>   pr

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > Hi,
> > 
> > While wireguard interfaces can have a description set by ifconfig, 
> > wireguard peers currently cannot. I now have a lot of peers and 
> > descriptions of them in ifconfig would be helpful.
> > 
> > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a 
> > corresponding '-wgdesc' option). Man page also updated.
> > 
> > NM
> 
> Now that my `ifconfig, wireguard output less verbose, unless -A or `
> diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> bump of an old thread.
> 
> Below is rebased on -current and tiny modified by me, Noah's diff.
> 
> You need both kernel and ifconfig with below code, otherwise you may see
> issues bringing up wg(4) interface. If you may loose access to machine
> behind wg(4) VPN, make sure you update on that machine both kernel and
> ifconfig(8) at the same time.
> 

Typo, s/wgpesc/wgdesc/


Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.457
diff -u -p -u -r1.457 ifconfig.c
--- sbin/ifconfig/ifconfig.c26 Oct 2022 17:06:31 -  1.457
+++ sbin/ifconfig/ifconfig.c10 Nov 2022 01:03:04 -
@@ -355,12 +355,14 @@ void  setwgpeerep(const char *, const cha
 void   setwgpeeraip(const char *, int);
 void   setwgpeerpsk(const char *, int);
 void   setwgpeerpka(const char *, int);
+void   setwgpeerdesc(const char *, int);
 void   setwgport(const char *, int);
 void   setwgkey(const char *, int);
 void   setwgrtable(const char *, int);
 
 void   unsetwgpeer(const char *, int);
 void   unsetwgpeerpsk(const char *, int);
+void   unsetwgpeerdesc(const char *, int);
 void   unsetwgpeerall(const char *, int);
 
 void   wg_status(int);
@@ -620,11 +622,13 @@ const struct  cmd {
{ "wgaip",  NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk",  NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka",  NEXTARG,A_WIREGUARD,setwgpeerpka},
+   { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey",  NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable",   NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0,  A_WIREGUARD,unsetwgpeerpsk},
+   { "-wgdesc",0,  A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0,  A_WIREGUARD,unsetwgpeerall},
 
 #else /* SMALL */
@@ -5843,6 +5847,16 @@ setwgpeerpka(const char *pka, int param)
 }
 
 void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+   if (wg_peer == NULL)
+   errx(1, "wgdesc: wgpeer not set");
+   if (strlen(wgdesc))
+   strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+   wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
 setwgport(const char *port, int param)
 {
const char *errmsg = NULL;
@@ -5889,6 +5903,15 @@ unsetwgpeerpsk(const char *value, int pa
 }
 
 void
+unsetwgpeerdesc(const char *value, int param)
+{
+   if (wg_peer == NULL)
+   errx(1, "wgdesc: wgpeer not set");
+   strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+   wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
 unsetwgpeerall(const char *value, int param)
 {
ensurewginterface();
@@ -5948,6 +5971,9 @@ wg_status(int ifaliases)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+   if (strlen(wg_peer->p_description))
+   printf("\t\twgdesc %s\n", 
wg_peer->p_description);
 
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
Index: share/man/man4/wg.4
===
RCS file: /cvs/src/share/man/man4/wg.4,v
retrieving revision 1.10
diff -u -p -u -r1.10 wg.4
--- share/man/man4/wg.4 14 Mar 2021 10:08:38 -  1.10
+++ share/man/man4/wg.4 10 Nov 2022 01:03:04 -
@@ -42,6 +42,19 @@ configuration file for
 .Xr netstart 8 .
 The interface itself can be configured with
 .Xr ifconfig 8 .
+To display
+.Cm wgpeer
+information for each
+.Nm wg
+interface option
+.Fl A
+to
+.Xr ifconfig 8
+should be used or
+.Nm wg
+interface should be specified as an argument to
+.Xr ifconfig 8
+command.

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> Hi,
> 
> While wireguard interfaces can have a description set by ifconfig, wireguard 
> peers currently cannot. I now have a lot of peers and descriptions of them in 
> ifconfig would be helpful.
> 
> This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a 
> corresponding '-wgdesc' option). Man page also updated.
> 
> NM

Now that my `ifconfig, wireguard output less verbose, unless -A or `
diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
bump of an old thread.

Below is rebased on -current and tiny modified by me, Noah's diff.

You need both kernel and ifconfig with below code, otherwise you may see
issues bringing up wg(4) interface. If you may loose access to machine
behind wg(4) VPN, make sure you update on that machine both kernel and
ifconfig(8) at the same time.


Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.457
diff -u -p -u -r1.457 ifconfig.c
--- sbin/ifconfig/ifconfig.c26 Oct 2022 17:06:31 -  1.457
+++ sbin/ifconfig/ifconfig.c5 Nov 2022 19:41:22 -
@@ -355,12 +355,14 @@ void  setwgpeerep(const char *, const cha
 void   setwgpeeraip(const char *, int);
 void   setwgpeerpsk(const char *, int);
 void   setwgpeerpka(const char *, int);
+void   setwgpeerdesc(const char *, int);
 void   setwgport(const char *, int);
 void   setwgkey(const char *, int);
 void   setwgrtable(const char *, int);
 
 void   unsetwgpeer(const char *, int);
 void   unsetwgpeerpsk(const char *, int);
+void   unsetwgpeerdesc(const char *, int);
 void   unsetwgpeerall(const char *, int);
 
 void   wg_status(int);
@@ -620,11 +622,13 @@ const struct  cmd {
{ "wgaip",  NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk",  NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka",  NEXTARG,A_WIREGUARD,setwgpeerpka},
+   { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey",  NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable",   NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0,  A_WIREGUARD,unsetwgpeerpsk},
+   { "-wgdesc",0,  A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0,  A_WIREGUARD,unsetwgpeerall},
 
 #else /* SMALL */
@@ -5843,6 +5847,16 @@ setwgpeerpka(const char *pka, int param)
 }
 
 void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+   if (wg_peer == NULL)
+   errx(1, "wgdesc: wgpeer not set");
+   if (strlen(wgdesc))
+   strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+   wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
 setwgport(const char *port, int param)
 {
const char *errmsg = NULL;
@@ -5889,6 +5903,15 @@ unsetwgpeerpsk(const char *value, int pa
 }
 
 void
+unsetwgpeerdesc(const char *value, int param)
+{
+   if (wg_peer == NULL)
+   errx(1, "wgpesc: wgpeer not set");
+   strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+   wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
 unsetwgpeerall(const char *value, int param)
 {
ensurewginterface();
@@ -5948,6 +5971,9 @@ wg_status(int ifaliases)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+   if (strlen(wg_peer->p_description))
+   printf("\t\twgdesc %s\n", 
wg_peer->p_description);
 
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
Index: share/man/man4/wg.4
===
RCS file: /cvs/src/share/man/man4/wg.4,v
retrieving revision 1.10
diff -u -p -u -r1.10 wg.4
--- share/man/man4/wg.4 14 Mar 2021 10:08:38 -  1.10
+++ share/man/man4/wg.4 5 Nov 2022 19:41:22 -
@@ -42,6 +42,19 @@ configuration file for
 .Xr netstart 8 .
 The interface itself can be configured with
 .Xr ifconfig 8 .
+To display
+.Cm wgpeer
+information for each
+.Nm wg
+interface option
+.Fl A
+to
+.Xr ifconfig 8
+should be used or
+.Nm wg
+interface should be specified as an argument to
+.Xr ifconfig 8
+command.
 .Pp
 .Nm wg
 interfaces support the following
Index: sys/net/if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.26
diff -u -p -u -r1.26 if_wg.c
--- sys/net/if_wg.c 21 Jul 2022 11:26:50 -  1.26
+++ sys/net/if_wg.c 5 Nov 2022 19:41:22 -
@@ -221,6 +221,9 @@ struct wg_peer {
 
SLIST_ENTRY(wg_peer) p_start_list;
int   

Re: ifconfig, wireguard output less verbose, unless -A or

[Mikolaj Kucharski]

Kind reminder. Below there is a comment with an OK from sthen@

Diff at the end of this email.


On Wed, Sep 07, 2022 at 05:29:38PM +0100, Stuart Henderson wrote:
> On 2022/09/07 15:25, Mikolaj Kucharski wrote:
> > Hi.
> > 
> > I didn't get a lof of feedback on this on the code level, however
> > got some intput on manual page changes. At the end of the email is
> > ifconfig.8 change from jmc@ and ifconfig.c from me.
> > 
> > 
> > On Sat, Sep 03, 2022 at 04:51:03PM +0100, Jason McIntyre wrote:
> > > On Sat, Sep 03, 2022 at 08:55:51AM +, Mikolaj Kucharski wrote:
> > > > Hi,
> > > > 
> > > > I tried to address what jmc@ mentioned below. I don't really know
> > > > mdoc(7) and English is not my native language, so I imagine there is
> > > > place for improvement in the wg(4) diff.
> > > > 
> > > 
> > > hi.
> > > 
> > > after looking again, i think maybe ifconfig.8 is the better place, but
> > > just not where it was originally proposed. by way of a peace offering,
> > > how about the diff below?
> > > 
> > > jmc
> > > 
> > [...]
> 
> It's all in ifndef SMALL so there are no ramdisk space concerns.
> Works as expected, I think it's a good idea. It's OK with me.
> 
> 
> > 
> > Index: ifconfig.c
> > ===
> > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> > retrieving revision 1.456
> > diff -u -p -u -r1.456 ifconfig.c
> > --- ifconfig.c  8 Jul 2022 07:04:54 -   1.456
> > +++ ifconfig.c  7 Sep 2022 15:18:50 -
> > @@ -363,7 +363,7 @@ voidunsetwgpeer(const char *, int);
> >  void   unsetwgpeerpsk(const char *, int);
> >  void   unsetwgpeerall(const char *, int);
> >  
> > -void   wg_status();
> > +void   wg_status(int);
> >  #else
> >  void   setignore(const char *, int);
> >  #endif
> > @@ -679,7 +679,7 @@ voidprintgroupattribs(char *);
> >  void   printif(char *, int);
> >  void   printb_status(unsigned short, unsigned char *);
> >  const char *get_linkstate(int, int);
> > -void   status(int, struct sockaddr_dl *, int);
> > +void   status(int, struct sockaddr_dl *, int, int);
> >  __dead voidusage(void);
> >  const char *get_string(const char *, const char *, u_int8_t *, int *);
> >  intlen_string(const u_int8_t *, int);
> > @@ -1195,7 +1195,7 @@ printif(char *name, int ifaliases)
> > continue;
> > ifdata = ifa->ifa_data;
> > status(1, (struct sockaddr_dl *)ifa->ifa_addr,
> > -   ifdata->ifi_link_state);
> > +   ifdata->ifi_link_state, ifaliases);
> > count++;
> > noinet = 1;
> > continue;
> > @@ -3316,7 +3316,7 @@ get_linkstate(int mt, int link_state)
> >   * specified, show it and it only; otherwise, show them all.
> >   */
> >  void
> > -status(int link, struct sockaddr_dl *sdl, int ls)
> > +status(int link, struct sockaddr_dl *sdl, int ls, int ifaliases)
> >  {
> > const struct afswtch *p = afp;
> > struct ifmediareq ifmr;
> > @@ -3391,7 +3391,7 @@ status(int link, struct sockaddr_dl *sdl
> > mpls_status();
> > pflow_status();
> > umb_status();
> > -   wg_status();
> > +   wg_status(ifaliases);
> >  #endif
> > trunk_status();
> > getifgroups();
> > @@ -5907,7 +5907,7 @@ process_wg_commands(void)
> >  }
> >  
> >  void
> > -wg_status(void)
> > +wg_status(int ifaliases)
> >  {
> > size_t   i, j, last_size;
> > struct timespec  now;
> > @@ -5942,45 +5942,47 @@ wg_status(void)
> > printf("\twgpubkey %s\n", key);
> > }
> >  
> > -   wg_peer = _interface->i_peers[0];
> > -   for (i = 0; i < wg_interface->i_peers_count; i++) {
> > -   b64_ntop(wg_peer->p_public, WG_KEY_LEN,
> > -   key, sizeof(key));
> > -   printf("\twgpeer %s\n", key);
> > -
> > -   if (wg_peer->p_flags & WG_PEER_HAS_PSK)
> > -   printf("\t\twgpsk (present)\n");
> > -
> > -   if (wg_peer->p_flags & WG_PEER_HAS_PKA && wg_peer->p_pka)
> > -   printf("\t\twgpka %u (sec)\n", wg_peer->p_pka

Re: rm -P and no-write on files - perm denied, bail out?

[Mikolaj Kucharski]

Hi,

Kind reminder. Diff re-attached at the end and on MARC:

https://marc.info/?l=openbsd-tech=166219807307308=2


On Sat, Sep 03, 2022 at 09:44:46AM +, Mikolaj Kucharski wrote:
> Hi,
> 
> I wanted to rm -rP some files on my disk and didn't notice that
> they lacked write permission for the user who executed rm(1)
> command.
> 
> $ echo foo > file-mode-444.txt
> $ chmod 0444 file-mode-444.txt
> $ ls -ln file-mode-444.txt
> -r--r--r--  1 5001  5001  4 Sep  3 09:36 file-mode-444.txt
> 
> $ rm -vfP file-mode-444.txt
> rm: file-mode-444.txt: Permission denied
> file-mode-444.txt
> $ echo $?
> 1
> 
> $ ls -l file-mode-444.txt
> ls: file-mode-444.txt: No such file or directory
> 
> I was not expecting this behaviour. My expectation was the file would
> NOT be removed. Hence the diff below:
> 
> 
> Index: rm.c
> ===
> RCS file: /cvs/src/bin/rm/rm.c,v
> retrieving revision 1.44
> diff -u -p -u -r1.44 rm.c
> --- rm.c  16 Aug 2022 13:52:41 -  1.44
> +++ rm.c  3 Sep 2022 09:37:44 -
> @@ -215,8 +215,11 @@ rm_tree(char **argv)
>   case FTS_F:
>   case FTS_NSOK:
>   if (Pflag)
> - rm_overwrite(p->fts_accpath, p->fts_info ==
> - FTS_NSOK ? NULL : p->fts_statp);
> + if (!rm_overwrite(p->fts_accpath, p->fts_info ==
> + FTS_NSOK ? NULL : p->fts_statp)) {
> + eval = 1;
> + continue;
> + }
>   /* FALLTHROUGH */
>   default:
>   if (!unlink(p->fts_accpath)) {
> @@ -267,7 +270,10 @@ rm_file(char **argv)
>   rval = rmdir(f);
>   else {
>   if (Pflag)
> - rm_overwrite(f, );
> + if (!rm_overwrite(f, )) {
> + eval = 1;
> + continue;
> + }
>   rval = unlink(f);
>   }
>   if (rval && (!fflag || errno != ENOENT)) {
> 
> 
> What do you guys think?
> 
> 
> $ ./obj/rm -vfP file-mode-444.txt
> rm: file-mode-444.txt: Permission denied
> $ echo $?
> 1
> 
> $ ls -ln file-mode-444.txt
> -r--r--r--  1 5001  5001  4 Sep  3 09:36 file-mode-444.txt
> 
> I did use `rm -fP` in the invocation, and reading the rm(1) manual page:
> 
>-f  Attempt to remove the files without prompting for confirmation,
>regardless of the file's permissions.  If the file does not
>exist, do not display a diagnostic message or modify the exit
>status to reflect an error.  The -f option overrides any previous
>-i options.
> 
> but not sure then what exactly should happen when -P and -f and no write
> permission.
> 


Index: rm.c
===
RCS file: /cvs/src/bin/rm/rm.c,v
retrieving revision 1.44
diff -u -p -u -r1.44 rm.c
--- rm.c16 Aug 2022 13:52:41 -  1.44
+++ rm.c14 Oct 2022 21:41:22 -
@@ -215,8 +215,11 @@ rm_tree(char **argv)
case FTS_F:
case FTS_NSOK:
if (Pflag)
-   rm_overwrite(p->fts_accpath, p->fts_info ==
-   FTS_NSOK ? NULL : p->fts_statp);
+   if (!rm_overwrite(p->fts_accpath, p->fts_info ==
+   FTS_NSOK ? NULL : p->fts_statp)) {
+   eval = 1;
+   continue;
+   }
/* FALLTHROUGH */
default:
if (!unlink(p->fts_accpath)) {
@@ -267,7 +270,10 @@ rm_file(char **argv)
rval = rmdir(f);
else {
if (Pflag)
-   rm_overwrite(f, );
+   if (!rm_overwrite(f, )) {
+   eval = 1;
+   continue;
+   }
rval = unlink(f);
}
if (rval && (!fflag || errno != ENOENT)) {


-- 
Regards,
 Mikolaj

Re: sysupgrade: exit 1 instead of exit 0 when ending early

[Mikolaj Kucharski]

On Mon, Oct 10, 2022 at 11:17:32AM -0600, Theo de Raadt wrote:
> It's been explained a few times that being up-to-date is not an error.
> It's a good thing, and no action is neccessary when up-to-date.

No action when up-to-date is not the same as action when not up-to-date,
when you use -n and additional own tooling around sysupgrade(8).


> Any non-zero value indicates an error, that would include 2.  You are
> marking this as an error, when it isn't.
> 
> You think this will help your scripting.  Do you not realize the proposed
> changes will break someone else's scripting?

I don't insist on this approach. I just propose a patch.

I do recognize that it is going to break someone else's workflow.

-- 
Regards,
 Mikolaj

Re: sysupgrade: exit 1 instead of exit 0 when ending early

[Mikolaj Kucharski]

On Fri, Oct 07, 2022 at 02:39:01PM -0400, Josh Grosse wrote:
> For ease of running sysupgrade from within a script.
> 

> diff --git a/usr.sbin/sysupgrade/sysupgrade.sh 
> b/usr.sbin/sysupgrade/sysupgrade.sh
> index d80ff127ffa..ce5800093c9 100644
> --- a/usr.sbin/sysupgrade/sysupgrade.sh
> +++ b/usr.sbin/sysupgrade/sysupgrade.sh
> @@ -153,7 +153,7 @@ rm SHA256.sig
>  
>  if cmp -s /var/db/installed.SHA256 SHA256 && ! $FORCE; then
>   echo "Already on latest snapshot."
> - exit 0
> + exit 1
>  fi
>  
>  # INSTALL.*, bsd*, *.tgz

Maybe something like this?


Index: sysupgrade.8
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
retrieving revision 1.13
diff -u -p -u -r1.13 sysupgrade.8
--- sysupgrade.88 Jun 2022 09:03:11 -   1.13
+++ sysupgrade.810 Oct 2022 06:59:49 -
@@ -89,6 +89,10 @@ mirror top-level URL for fetching an upg
 .It Pa /home/_sysupgrade
 Directory the upgrade is downloaded to.
 .El
+.Sh EXIT STATUS
+.Ex -std sysupgrade
+In particular, 2 indicates upgraded was requested but
+system is already on the latest snapshot.
 .Sh SEE ALSO
 .Xr signify 1 ,
 .Xr installurl 5 ,
Index: sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.48
diff -u -p -u -r1.48 sysupgrade.sh
--- sysupgrade.sh   8 Jun 2022 09:03:11 -   1.48
+++ sysupgrade.sh   10 Oct 2022 06:59:49 -
@@ -153,7 +153,7 @@ rm SHA256.sig
 
 if cmp -s /var/db/installed.SHA256 SHA256 && ! $FORCE; then
echo "Already on latest snapshot."
-   exit 0
+   exit 2
 fi
 
 # INSTALL.*, bsd*, *.tgz


-- 
Regards,
 Mikolaj

Re: sysupgrade: exit 1 instead of exit 0 when ending early

[Mikolaj Kucharski]

On Sat, Oct 08, 2022 at 09:57:54AM +, Klemens Nanni wrote:
> On Sat, Oct 08, 2022 at 07:33:38AM +0200, Florian Obser wrote:
> > Being up2date doesn't feel like an error to me, what am I missing?
> 
> I concur.
> 

I think Josh is looking for similar behaviour to syspatch(8). To be able
to distinguish, upgrade is ready (exit 0), system is up to date (exit 2)
or error occurred (exit 1).

I have the same need to distinguish between nothing to do - you are on
latest snapshot (sysupgrade -s -n) versus pending upgrade, as I use
`sysupgrade -s -n` a lot.

On my side I just check that non-zero /bsd.upgrade file exists. I
think what Josh is trying to achieve make sense, but I would make it
exit code 2, the same way like syspatch(8).

-- 
Regards,
 Mikolaj

Re: Softraid crypto with keydisk and installboot, skip on the same disk

[Mikolaj Kucharski]

On Sun, Oct 02, 2022 at 09:09:36PM +, Klemens Nanni wrote:
> On Tue, Sep 06, 2022 at 09:06:41PM +, Klemens Nanni wrote:
> > On Sun, Sep 04, 2022 at 07:08:51PM +, Mikolaj Kucharski wrote:
> > > Hi,
> > > 
> > > I have strange setup on some of my machines, when I want to encrypt disk
> > > where OpenBSD is installed, but still be able to boot them up without
> > > any user interaction, like passphrase entry for CRYPTO softraid(4). I
> > > have this so I can with little time spent lock out access to the disk,
> > > by wiping beginning of the disk, instead of entire disk. I do recognise
> > > magnitute of limitations of this. I still try to wipe entire disk, when
> > > it's time for a machine decommission, but first I break CRYPTO softraid
> > > by wiping beginning and then switch to proper full disk wipe.
> > > 
> > > All in all that brings me to the below diff. I was only able to test on
> > > amd64, as this is the only type of machine which I have.
> > 
> > Thanks, although the setup seems a bit strange, your diff makes sense
> > and works as advertised on amd64, arm64 and sparc64.
> > 
> > I have adjusted our installboot regress tests to install onto softraid
> > RAID 1C with a keydisk so it must a) iterate over multiple chunks and
> > b) ignore the key-disk, which is a nice combined exercise.
> > 
> > Here is your diff with tweaked wording so it is clearer;  this also
> > nicely aligns the "- skipping..." for both offline and keydisk cases.
> > 
> > With this diff, regress/usr.sbin/installboot passes on amd64, arm64 and
> > sparc64 using the above mentioned softraid.
> > 
> > regress uses a separate device for the keydisk but that does not effect
> > the skip logic.
> > 
> > Feedback? OK?
> 
> Ping.

I didn't had time to test it again, like I did few months back with my
diff, but reading below diff and comparing to mine, looks exactly the
same (the i386 part, as that what was in my diff originally).

My only nit would be the grammar in the comment, like also mentioned
by Raf Czlonka.


> Index: efi_softraid.c
> ===
> RCS file: /cvs/src/usr.sbin/installboot/efi_softraid.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 efi_softraid.c
> --- efi_softraid.c29 Aug 2022 18:54:43 -  1.2
> +++ efi_softraid.c2 Oct 2022 21:08:14 -
> @@ -54,6 +54,13 @@ sr_install_bootblk(int devfd, int vol, i
>   return;
>   }
>  
> + /* Keydisks always has as size of zero. */
> + if (bd.bd_size == 0) {
> + fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
> + disk);
> + return;
> + }
> +
>   if (strlen(bd.bd_vendor) < 1)
>   errx(1, "invalid disk name");
>   part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
> Index: i386_softraid.c
> ===
> RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 i386_softraid.c
> --- i386_softraid.c   29 Aug 2022 18:54:43 -  1.19
> +++ i386_softraid.c   2 Oct 2022 21:08:15 -
> @@ -65,6 +65,13 @@ sr_install_bootblk(int devfd, int vol, i
>   return;
>   }
>  
> + /* Keydisks always has as size of zero. */
> + if (bd.bd_size == 0) {
> + fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
> + disk);
> + return;
> + }
> +
>   if (strlen(bd.bd_vendor) < 1)
>   errx(1, "invalid disk name");
>   part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
> Index: sparc64_softraid.c
> ===
> RCS file: /cvs/src/usr.sbin/installboot/sparc64_softraid.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 sparc64_softraid.c
> --- sparc64_softraid.c29 Aug 2022 18:54:43 -  1.6
> +++ sparc64_softraid.c2 Oct 2022 21:08:16 -
> @@ -55,6 +55,13 @@ sr_install_bootblk(int devfd, int vol, i
>   return;
>   }
>  
> + /* Keydisks always has as size of zero. */
> + if (bd.bd_size == 0) {
> + fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
> + disk);
> + return;
> + }
> +
>   if (strlen(bd.bd_vendor) < 1)
>   errx(1, "invalid disk name");
>   part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];

-- 
Regards,
 Mikolaj

Re: ifconfig, wireguard output less verbose, unless -A or

[Mikolaj Kucharski]

Kind reminder.

Below diff at https://marc.info/?l=openbsd-tech=166256415030704=2


On Wed, Sep 07, 2022 at 03:25:58PM +, Mikolaj Kucharski wrote:
> Hi.
> 
> I didn't get a lof of feedback on this on the code level, however
> got some intput on manual page changes. At the end of the email is
> ifconfig.8 change from jmc@ and ifconfig.c from me.
> 
> 
> On Sat, Sep 03, 2022 at 04:51:03PM +0100, Jason McIntyre wrote:
> > On Sat, Sep 03, 2022 at 08:55:51AM +, Mikolaj Kucharski wrote:
> > > Hi,
> > > 
> > > I tried to address what jmc@ mentioned below. I don't really know
> > > mdoc(7) and English is not my native language, so I imagine there is
> > > place for improvement in the wg(4) diff.
> > > 
> > 
> > hi.
> > 
> > after looking again, i think maybe ifconfig.8 is the better place, but
> > just not where it was originally proposed. by way of a peace offering,
> > how about the diff below?
> > 
> > jmc
> > 
> [...]
> 
> 
> Index: ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.456
> diff -u -p -u -r1.456 ifconfig.c
> --- ifconfig.c8 Jul 2022 07:04:54 -   1.456
> +++ ifconfig.c7 Sep 2022 15:18:50 -
> @@ -363,7 +363,7 @@ void  unsetwgpeer(const char *, int);
>  void unsetwgpeerpsk(const char *, int);
>  void unsetwgpeerall(const char *, int);
>  
> -void wg_status();
> +void wg_status(int);
>  #else
>  void setignore(const char *, int);
>  #endif
> @@ -679,7 +679,7 @@ void  printgroupattribs(char *);
>  void printif(char *, int);
>  void printb_status(unsigned short, unsigned char *);
>  const char *get_linkstate(int, int);
> -void status(int, struct sockaddr_dl *, int);
> +void status(int, struct sockaddr_dl *, int, int);
>  __dead void  usage(void);
>  const char *get_string(const char *, const char *, u_int8_t *, int *);
>  int  len_string(const u_int8_t *, int);
> @@ -1195,7 +1195,7 @@ printif(char *name, int ifaliases)
>   continue;
>   ifdata = ifa->ifa_data;
>   status(1, (struct sockaddr_dl *)ifa->ifa_addr,
> - ifdata->ifi_link_state);
> + ifdata->ifi_link_state, ifaliases);
>   count++;
>   noinet = 1;
>   continue;
> @@ -3316,7 +3316,7 @@ get_linkstate(int mt, int link_state)
>   * specified, show it and it only; otherwise, show them all.
>   */
>  void
> -status(int link, struct sockaddr_dl *sdl, int ls)
> +status(int link, struct sockaddr_dl *sdl, int ls, int ifaliases)
>  {
>   const struct afswtch *p = afp;
>   struct ifmediareq ifmr;
> @@ -3391,7 +3391,7 @@ status(int link, struct sockaddr_dl *sdl
>   mpls_status();
>   pflow_status();
>   umb_status();
> - wg_status();
> + wg_status(ifaliases);
>  #endif
>   trunk_status();
>   getifgroups();
> @@ -5907,7 +5907,7 @@ process_wg_commands(void)
>  }
>  
>  void
> -wg_status(void)
> +wg_status(int ifaliases)
>  {
>   size_t   i, j, last_size;
>   struct timespec  now;
> @@ -5942,45 +5942,47 @@ wg_status(void)
>   printf("\twgpubkey %s\n", key);
>   }
>  
> - wg_peer = _interface->i_peers[0];
> - for (i = 0; i < wg_interface->i_peers_count; i++) {
> - b64_ntop(wg_peer->p_public, WG_KEY_LEN,
> - key, sizeof(key));
> - printf("\twgpeer %s\n", key);
> -
> - if (wg_peer->p_flags & WG_PEER_HAS_PSK)
> - printf("\t\twgpsk (present)\n");
> -
> - if (wg_peer->p_flags & WG_PEER_HAS_PKA && wg_peer->p_pka)
> - printf("\t\twgpka %u (sec)\n", wg_peer->p_pka);
> -
> - if (wg_peer->p_flags & WG_PEER_HAS_ENDPOINT) {
> - if (getnameinfo(_peer->p_sa, wg_peer->p_sa.sa_len,
> - hbuf, sizeof(hbuf), sbuf, sizeof(sbuf),
> - NI_NUMERICHOST | NI_NUMERICSERV) == 0)
> - printf("\t\twgendpoint %s %s\n", hbuf, sbuf);
> - else
> - printf("\t\twgendpoint unable to print\n");
> - }
> + if (ifaliases) {
> + wg_peer = _interface->i_peers[0];
> + for (i = 0; i < wg_interface->i_peers_count; i++) {
> + b64_ntop(wg_peer-

Re: ifconfig, wireguard output less verbose, unless -A or

[Mikolaj Kucharski]

Hi.

I didn't get a lof of feedback on this on the code level, however
got some intput on manual page changes. At the end of the email is
ifconfig.8 change from jmc@ and ifconfig.c from me.


On Sat, Sep 03, 2022 at 04:51:03PM +0100, Jason McIntyre wrote:
> On Sat, Sep 03, 2022 at 08:55:51AM +0000, Mikolaj Kucharski wrote:
> > Hi,
> > 
> > I tried to address what jmc@ mentioned below. I don't really know
> > mdoc(7) and English is not my native language, so I imagine there is
> > place for improvement in the wg(4) diff.
> > 
> 
> hi.
> 
> after looking again, i think maybe ifconfig.8 is the better place, but
> just not where it was originally proposed. by way of a peace offering,
> how about the diff below?
> 
> jmc
> 
[...]


Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.456
diff -u -p -u -r1.456 ifconfig.c
--- ifconfig.c  8 Jul 2022 07:04:54 -   1.456
+++ ifconfig.c  7 Sep 2022 15:18:50 -
@@ -363,7 +363,7 @@ voidunsetwgpeer(const char *, int);
 void   unsetwgpeerpsk(const char *, int);
 void   unsetwgpeerall(const char *, int);
 
-void   wg_status();
+void   wg_status(int);
 #else
 void   setignore(const char *, int);
 #endif
@@ -679,7 +679,7 @@ voidprintgroupattribs(char *);
 void   printif(char *, int);
 void   printb_status(unsigned short, unsigned char *);
 const char *get_linkstate(int, int);
-void   status(int, struct sockaddr_dl *, int);
+void   status(int, struct sockaddr_dl *, int, int);
 __dead voidusage(void);
 const char *get_string(const char *, const char *, u_int8_t *, int *);
 intlen_string(const u_int8_t *, int);
@@ -1195,7 +1195,7 @@ printif(char *name, int ifaliases)
continue;
ifdata = ifa->ifa_data;
status(1, (struct sockaddr_dl *)ifa->ifa_addr,
-   ifdata->ifi_link_state);
+   ifdata->ifi_link_state, ifaliases);
count++;
noinet = 1;
continue;
@@ -3316,7 +3316,7 @@ get_linkstate(int mt, int link_state)
  * specified, show it and it only; otherwise, show them all.
  */
 void
-status(int link, struct sockaddr_dl *sdl, int ls)
+status(int link, struct sockaddr_dl *sdl, int ls, int ifaliases)
 {
const struct afswtch *p = afp;
struct ifmediareq ifmr;
@@ -3391,7 +3391,7 @@ status(int link, struct sockaddr_dl *sdl
mpls_status();
pflow_status();
umb_status();
-   wg_status();
+   wg_status(ifaliases);
 #endif
trunk_status();
getifgroups();
@@ -5907,7 +5907,7 @@ process_wg_commands(void)
 }
 
 void
-wg_status(void)
+wg_status(int ifaliases)
 {
size_t   i, j, last_size;
struct timespec  now;
@@ -5942,45 +5942,47 @@ wg_status(void)
printf("\twgpubkey %s\n", key);
}
 
-   wg_peer = _interface->i_peers[0];
-   for (i = 0; i < wg_interface->i_peers_count; i++) {
-   b64_ntop(wg_peer->p_public, WG_KEY_LEN,
-   key, sizeof(key));
-   printf("\twgpeer %s\n", key);
-
-   if (wg_peer->p_flags & WG_PEER_HAS_PSK)
-   printf("\t\twgpsk (present)\n");
-
-   if (wg_peer->p_flags & WG_PEER_HAS_PKA && wg_peer->p_pka)
-   printf("\t\twgpka %u (sec)\n", wg_peer->p_pka);
-
-   if (wg_peer->p_flags & WG_PEER_HAS_ENDPOINT) {
-   if (getnameinfo(_peer->p_sa, wg_peer->p_sa.sa_len,
-   hbuf, sizeof(hbuf), sbuf, sizeof(sbuf),
-   NI_NUMERICHOST | NI_NUMERICSERV) == 0)
-   printf("\t\twgendpoint %s %s\n", hbuf, sbuf);
-   else
-   printf("\t\twgendpoint unable to print\n");
-   }
+   if (ifaliases) {
+   wg_peer = _interface->i_peers[0];
+   for (i = 0; i < wg_interface->i_peers_count; i++) {
+   b64_ntop(wg_peer->p_public, WG_KEY_LEN,
+   key, sizeof(key));
+   printf("\twgpeer %s\n", key);
+
+   if (wg_peer->p_flags & WG_PEER_HAS_PSK)
+   printf("\t\twgpsk (present)\n");
+
+   if (wg_peer->p_flags & WG_PEER_HAS_PKA && 
wg_peer->p_pka)
+   printf("\t\twgpka %u (sec)\n", wg_peer->p_pka);
+
+   if (wg_peer->p_flags & WG_PEER_HAS_ENDPOINT) {
+   if (getnameinfo(_pe

Re: Softraid crypto with keydisk and installboot, skip on the same disk

[Mikolaj Kucharski]

On Tue, Sep 06, 2022 at 09:06:41PM +, Klemens Nanni wrote:
> On Sun, Sep 04, 2022 at 07:08:51PM +0000, Mikolaj Kucharski wrote:
> > Hi,
> > 
> > I have strange setup on some of my machines, when I want to encrypt disk
> > where OpenBSD is installed, but still be able to boot them up without
> > any user interaction, like passphrase entry for CRYPTO softraid(4). I
> > have this so I can with little time spent lock out access to the disk,
> > by wiping beginning of the disk, instead of entire disk. I do recognise
> > magnitute of limitations of this. I still try to wipe entire disk, when
> > it's time for a machine decommission, but first I break CRYPTO softraid
> > by wiping beginning and then switch to proper full disk wipe.
> > 
> > All in all that brings me to the below diff. I was only able to test on
> > amd64, as this is the only type of machine which I have.
> 
> Thanks, although the setup seems a bit strange, your diff makes sense
> and works as advertised on amd64, arm64 and sparc64.
> 
> I have adjusted our installboot regress tests to install onto softraid
> RAID 1C with a keydisk so it must a) iterate over multiple chunks and
> b) ignore the key-disk, which is a nice combined exercise.
> 
> Here is your diff with tweaked wording so it is clearer;  this also
> nicely aligns the "- skipping..." for both offline and keydisk cases.
> 
> With this diff, regress/usr.sbin/installboot passes on amd64, arm64 and
> sparc64 using the above mentioned softraid.
> 
> regress uses a separate device for the keydisk but that does not effect
> the skip logic.
> 
> Feedback? OK?

Thank you! One question about source code comment and English language.

> > 
> > 
> > Index: i386_softraid.c
> > ===
> > RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
> > retrieving revision 1.19
> > diff -u -p -u -r1.19 i386_softraid.c
> > --- i386_softraid.c 29 Aug 2022 18:54:43 -  1.19
> > +++ i386_softraid.c 3 Sep 2022 11:28:55 -
> > @@ -65,6 +65,13 @@ sr_install_bootblk(int devfd, int vol, i
> > return;
> > }
> >  
> > +   /* Key disk has size of zero */
> > +   if (bd.bd_size == 0) {
> > +   fprintf(stderr, "softraid chunk %u looks like key disk - "
> > +   "skipping...\n", disk);
> > +   return;
> > +   }
> > +
> > if (strlen(bd.bd_vendor) < 1)
> > errx(1, "invalid disk name");
> > part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
> > 
> > 
> > Below follows my test and comments what happens without the diff
> > and with the diff.
> > 
> > First without the diff machine doesn't boot when I use keydisk on the
> > same disk which has the OpenBSD operaring system, wd0a and wd0d:
> > 
> > Booting from Hard Disk...
> > Using drive 0, partition 3.
> > Loading..
> > ERR M
> > 
> > 
> > To keep it short, it is because of installboot(8) installs boot blocks
> > on both wd0a and wd0d:
> > 
> > ramdisk# bioctl sd0
> > Volume  Status   Size Device
> > softraid0 0 Online   268426960384 sd0 CRYPTO
> >   0 Online   268426960384 0:0.0   noencl 
> >   1 Online   key disk 0:1.0   noencl 
> 
> 
> Index: efi_softraid.c
> ===
> RCS file: /cvs/src/usr.sbin/installboot/efi_softraid.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 efi_softraid.c
> --- efi_softraid.c29 Aug 2022 18:54:43 -  1.2
> +++ efi_softraid.c6 Sep 2022 20:47:16 -
> @@ -54,6 +54,13 @@ sr_install_bootblk(int devfd, int vol, i
>   return;
>   }
>  
> + /* Keydisks always has as size of zero. */

I'm not good with words, but is this correct grammar?


> + if (bd.bd_size == 0) {
> + fprintf(stderr, "softraid chunk %u is keydisk - skipping...\n",
> + disk);
> + return;
> + }
> +
>   if (strlen(bd.bd_vendor) < 1)
>   errx(1, "invalid disk name");
>   part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];
> Index: i386_softraid.c
> ===
> RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 i386_softraid.c
> --- i386_softraid.c   29 Aug 2022 18:54:43 -  1.19
> +++ i386_softraid.c   6 Sep 2022 20:47:19 -
> @@ -65,6 +65,13 @@ sr_insta

Re: Softraid crypto with keydisk and installboot, skip on the same disk

[Mikolaj Kucharski]

On Sun, Sep 04, 2022 at 11:25:08PM +, Mikolaj Kucharski wrote:
> # bioctl sd0
> Volume  Status   Size Device  
> softraid0 0 Online   268426960384 sd0 CRYPTO
>   0 Online   268426960384 0:0.0   noencl 
>   1 Online   key disk 0:1.0   noencl 

...

> keydisk# installboot -r /mnt -nv sd0
> Using /mnt as root
> would install bootstrap on /dev/rsd0c
> using first-stage /mnt/usr/mdec/biosboot, second-stage /mnt/usr/mdec/boot
> sd0: softraid volume with 2 disk(s)
> sd0: would install boot loader on softraid volume
> /mnt/usr/mdec/boot is 6 blocks x 16384 bytes
> wd0a: would install boot blocks on /dev/rwd0c, part offset 144
> master boot record (MBR) at sector 0
> partition 3: type 0xA6 offset 64 size 524287936
> /mnt/usr/mdec/biosboot will be written at sector 64
> wd0d: would install boot blocks on /dev/rwd0c, part offset 524272079
> master boot record (MBR) at sector 0
> partition 3: type 0xA6 offset 64 size 524287936
> /mnt/usr/mdec/biosboot will be written at sector 64
> keydisk# reboot
> 
> 
> Press ESC for boot menu.
> 
> Booting from Hard Disk...
> Using drive 0, partition 3.
> Loading..
> ERR M

Ah no. It make sense why it doesn't boot. Because key disk is always
last on the list of chunks and that means no matter what is the order
on wd0, installboot will install boot blocks on key disk always last,
so last chunk (key disk) always wins, hence boot failure.

-- 
Regards,
 Mikolaj

Re: Softraid crypto with keydisk and installboot, skip on the same disk

[Mikolaj Kucharski]

On Sun, Sep 04, 2022 at 10:21:24PM +, Klemens Nanni wrote:
...
> > Booting from Hard Disk...
> > Using drive 0, partition 3.
> > Loading..
> > ERR M
> > 
> > 
> > To keep it short, it is because of installboot(8) installs boot blocks
> > on both wd0a and wd0d:
> > 
> > ramdisk# bioctl sd0
> > Volume  Status   Size Device
> > softraid0 0 Online   268426960384 sd0 CRYPTO
> >   0 Online   268426960384 0:0.0   noencl 
> >   1 Online   key disk 0:1.0   noencl 
> 
> So CRYPTO on wd0d and keydisk on wd0a...

Correct.

> > 
> > ramdisk# installboot -r /mnt -nv sd0
> > Using /mnt as root
> > would install bootstrap on /dev/rsd0c
> > using first-stage /mnt/usr/mdec/biosboot, second-stage
> > /mnt/usr/mdec/boot
> > sd0: softraid volume with 2 disk(s)
> > sd0: would install boot loader on softraid volume
> > /mnt/usr/mdec/boot is 6 blocks x 16384 bytes
> > wd0d: would install boot blocks on /dev/rwd0c, part offset 16145
> > master boot record (MBR) at sector 0
> > partition 3: type 0xA6 offset 64 size 524287936
> > /mnt/usr/mdec/biosboot will be written at sector 64
> > wd0a: would install boot blocks on /dev/rwd0c, part offset 144
> > master boot record (MBR) at sector 0
> > partition 3: type 0xA6 offset 64 size 524287936
> > /mnt/usr/mdec/biosboot will be written at sector 64
> > 
> > We see above that sd0 is softraid volume with 2 disks and then boot
> > blocks is installed on wd0d and wd0a. With my change boot blocks are
> > only installed on wd0d and wd0a (keydisk) is skipped.
> 
> ... and installboot writes to the keydisk, making it bootable.
> 
> Is this failing to boot for you because wd0a is tried first but fails
> because it is a keydisk?  Put differently, would your questionable
> setup boot without a diff if CRYPTO was on wd0a and the keydisk on wd0d?

I don't know. I'm puzzled. I just tested with the other way around:

# bioctl sd0
Volume  Status   Size Device  
softraid0 0 Online   268426960384 sd0 CRYPTO
  0 Online   268426960384 0:0.0   noencl 
  1 Online   key disk 0:1.0   noencl 

and installboot(8) did each device in different order:

# installboot -r /mnt -nv sd0
Using /mnt as root
would install bootstrap on /dev/rsd0c
using first-stage /mnt/usr/mdec/biosboot, second-stage
/mnt/usr/mdec/boot
sd0: softraid volume with 2 disk(s)
sd0: would install boot loader on softraid volume
/mnt/usr/mdec/boot is 6 blocks x 16384 bytes
wd0a: would install boot blocks on /dev/rwd0c, part offset 144
master boot record (MBR) at sector 0
partition 3: type 0xA6 offset 64 size 524287936
/mnt/usr/mdec/biosboot will be written at sector 64
wd0d: would install boot blocks on /dev/rwd0c, part offset 524272079
master boot record (MBR) at sector 0
partition 3: type 0xA6 offset 64 size 524287936
/mnt/usr/mdec/biosboot will be written at sector 64

so far, this is what I expected, that installboot would iterate each
disk as they are shown in bioctl(8) output, wd0a first, wd0d second.
However, surprisingly to me, machine still didn't boot.

Press ESC for boot menu.

Booting from Hard Disk...
Using drive 0, partition 3.
Loading..
ERR M


Full log at the end of this email (without OpenBSD install itself, which is 
typical).


> Seing installboot write to keydisks seems unexpected, I'd say it should
> always ignore them.
> 
> Are keydisks expected to be present after unlocking the volume?
> I'm picturing users having a USB stick they put in temporarily to unlock
> and then boot/run the system without the stick plugged in.
> 
> But installboot seems to expect keydisks to be present, which is really
> odd.  It means you can't install bootstraps on your currently unlocked
> crypto volume without having the keydisk available...

I don't think installboot expects keydisks to be present. I don't think
it cares. It just iterates over softraid(4) chunks which are online.

> That does not sound right, but I'm not a keydisk user myself, so I have
> to test/dig around a little.
> 


I'm not sure did I made any mistake below, from what Klemens wanted to know.



erase ^?, werase ^W, kill ^U, intr ^C, status ^T

Welcome to the OpenBSD/amd64 7.2 installation program.
(I)nstall, (U)pgrade, (A)utoinstall or (S)hell? s
# sysctl -n hw.disknames
wd0:ccb72943e9945c9c,rd0:73efb2b48b898ab3
# cd /dev
# sh MAKEDEV wd0 wd1 wd2 wd3
# sh MAKEDEV sd0 sd1 sd2 sd3
# dd if=/dev/zero bs=1024 count=10240 of=/dev/rwd0c
10240+0 records in
10240+0 records out
10485760 bytes transferred in 4.955 secs (2115830 bytes/sec)
# fdisk -iy wd0
Writing MBR at offset 0.
# disklabel -E wd0
Label editor (enter '?' for help at any prompt)
wd0> p g
OpenBSD area: 64-524288000; size: 250.0G; free: 250.0G
#size   offset  fstype [fsize bsize   cpg]
  c:   250.0G0  unused
wd0> a a
offset: [64] 
size: [524287936] 524271935
FS type: [4.2BSD] RAID
wd0*> a d
offset: [524271999] 

Softraid crypto with keydisk and installboot, skip on the same disk

[Mikolaj Kucharski]

Hi,

I have strange setup on some of my machines, when I want to encrypt disk
where OpenBSD is installed, but still be able to boot them up without
any user interaction, like passphrase entry for CRYPTO softraid(4). I
have this so I can with little time spent lock out access to the disk,
by wiping beginning of the disk, instead of entire disk. I do recognise
magnitute of limitations of this. I still try to wipe entire disk, when
it's time for a machine decommission, but first I break CRYPTO softraid
by wiping beginning and then switch to proper full disk wipe.

All in all that brings me to the below diff. I was only able to test on
amd64, as this is the only type of machine which I have.


Index: i386_softraid.c
===
RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
retrieving revision 1.19
diff -u -p -u -r1.19 i386_softraid.c
--- i386_softraid.c 29 Aug 2022 18:54:43 -  1.19
+++ i386_softraid.c 3 Sep 2022 11:28:55 -
@@ -65,6 +65,13 @@ sr_install_bootblk(int devfd, int vol, i
return;
}
 
+   /* Key disk has size of zero */
+   if (bd.bd_size == 0) {
+   fprintf(stderr, "softraid chunk %u looks like key disk - "
+   "skipping...\n", disk);
+   return;
+   }
+
if (strlen(bd.bd_vendor) < 1)
errx(1, "invalid disk name");
part = bd.bd_vendor[strlen(bd.bd_vendor) - 1];


Below follows my test and comments what happens without the diff
and with the diff.

First without the diff machine doesn't boot when I use keydisk on the
same disk which has the OpenBSD operaring system, wd0a and wd0d:

Booting from Hard Disk...
Using drive 0, partition 3.
Loading..
ERR M


To keep it short, it is because of installboot(8) installs boot blocks
on both wd0a and wd0d:

ramdisk# bioctl sd0
Volume  Status   Size Device
softraid0 0 Online   268426960384 sd0 CRYPTO
  0 Online   268426960384 0:0.0   noencl 
  1 Online   key disk 0:1.0   noencl 


ramdisk# installboot -r /mnt -nv sd0
Using /mnt as root
would install bootstrap on /dev/rsd0c
using first-stage /mnt/usr/mdec/biosboot, second-stage
/mnt/usr/mdec/boot
sd0: softraid volume with 2 disk(s)
sd0: would install boot loader on softraid volume
/mnt/usr/mdec/boot is 6 blocks x 16384 bytes
wd0d: would install boot blocks on /dev/rwd0c, part offset 16145
master boot record (MBR) at sector 0
partition 3: type 0xA6 offset 64 size 524287936
/mnt/usr/mdec/biosboot will be written at sector 64
wd0a: would install boot blocks on /dev/rwd0c, part offset 144
master boot record (MBR) at sector 0
partition 3: type 0xA6 offset 64 size 524287936
/mnt/usr/mdec/biosboot will be written at sector 64

We see above that sd0 is softraid volume with 2 disks and then boot
blocks is installed on wd0d and wd0a. With my change boot blocks are
only installed on wd0d and wd0a (keydisk) is skipped.


ramdisk# cd /mnt
ramdisk# ftp https://example.com/installboot.bin
Trying 10.123.123.123...
Requesting https://example.com/installboot.bin
149040 bytes received in 0.21 seconds (671.19 KB/s)
ramdisk# chmod 0755 /mnt/installboot.bin
ramdisk# /mnt/installboot.bin -r /mnt -nv sd0
Using /mnt as root
would install bootstrap on /dev/rsd0c
using first-stage /mnt/usr/mdec/biosboot, second-stage /mnt/usr/mdec/boot
sd0: softraid volume with 2 disk(s)
sd0: would install boot loader on softraid volume
/mnt/usr/mdec/boot is 6 blocks x 16384 bytes
wd0d: would install boot blocks on /dev/rwd0c, part offset 16145
master boot record (MBR) at sector 0
partition 3: type 0xA6 offset 64 size 524287936
/mnt/usr/mdec/biosboot will be written at sector 64
softraid chunk 1 looks like key disk - skipping...


Here is full installation log on OpenBSD -current when installaton is
not bootable with above ERR M error.


root on rd0a swap on rd0b dump on rd0b
erase ^?, werase ^W, kill ^U, intr ^C, status ^T

Welcome to the OpenBSD/amd64 7.2 installation program.
(I)nstall, (U)pgrade, (A)utoinstall or (S)hell? s
# cd /dev
# sysctl -n hw.disknames
wd0:ccb72943e9945c9c,rd0:73efb2b48b898ab3
# sh MAKEDEV wd0 wd1 wd2 wd3
# sh MAKEDEV sd0 sd1 sd2 sd3
# dd if=/dev/zero bs=1024 count=10240 of=/dev/rwd0c
10240+0 records in
10240+0 records out
10485760 bytes transferred in 5.606 secs (1870132 bytes/sec)
# fdisk -iy wd0
Writing MBR at offset 0.
# disklabel -E wd0
Label editor (enter '?' for help at any prompt)
wd0> p g
OpenBSD area: 64-524288000; size: 250.0G; free: 250.0G
#size   offset  fstype [fsize bsize   cpg]
  c:   250.0G0  unused
wd0> a a
offset: [64]
size: [524287936] 4m
FS type: [4.2BSD] RAID
wd0*> a d
offset: [16065]
size: [524271935] *
FS type: [4.2BSD] RAID
wd0*> w
wd0> q
No label changes.
# dd if=/dev/zero bs=1024 count=10240 of=/dev/rwd0d
10240+0 records in
10240+0 records out
10485760 bytes transferred in 4.258 secs 

rm -P and no-write on files - perm denied, bail out?

[Mikolaj Kucharski]

Hi,

I wanted to rm -rP some files on my disk and didn't notice that
they lacked write permission for the user who executed rm(1)
command.

$ echo foo > file-mode-444.txt
$ chmod 0444 file-mode-444.txt
$ ls -ln file-mode-444.txt
-r--r--r--  1 5001  5001  4 Sep  3 09:36 file-mode-444.txt

$ rm -vfP file-mode-444.txt
rm: file-mode-444.txt: Permission denied
file-mode-444.txt
$ echo $?
1

$ ls -l file-mode-444.txt
ls: file-mode-444.txt: No such file or directory

I was not expecting this behaviour. My expectation was the file would
NOT be removed. Hence the diff below:


Index: rm.c
===
RCS file: /cvs/src/bin/rm/rm.c,v
retrieving revision 1.44
diff -u -p -u -r1.44 rm.c
--- rm.c16 Aug 2022 13:52:41 -  1.44
+++ rm.c3 Sep 2022 09:37:44 -
@@ -215,8 +215,11 @@ rm_tree(char **argv)
case FTS_F:
case FTS_NSOK:
if (Pflag)
-   rm_overwrite(p->fts_accpath, p->fts_info ==
-   FTS_NSOK ? NULL : p->fts_statp);
+   if (!rm_overwrite(p->fts_accpath, p->fts_info ==
+   FTS_NSOK ? NULL : p->fts_statp)) {
+   eval = 1;
+   continue;
+   }
/* FALLTHROUGH */
default:
if (!unlink(p->fts_accpath)) {
@@ -267,7 +270,10 @@ rm_file(char **argv)
rval = rmdir(f);
else {
if (Pflag)
-   rm_overwrite(f, );
+   if (!rm_overwrite(f, )) {
+   eval = 1;
+   continue;
+   }
rval = unlink(f);
}
if (rval && (!fflag || errno != ENOENT)) {


What do you guys think?


$ ./obj/rm -vfP file-mode-444.txt
rm: file-mode-444.txt: Permission denied
$ echo $?
1

$ ls -ln file-mode-444.txt
-r--r--r--  1 5001  5001  4 Sep  3 09:36 file-mode-444.txt

I did use `rm -fP` in the invocation, and reading the rm(1) manual page:

   -f  Attempt to remove the files without prompting for confirmation,
   regardless of the file's permissions.  If the file does not
   exist, do not display a diagnostic message or modify the exit
   status to reflect an error.  The -f option overrides any previous
   -i options.

but not sure then what exactly should happen when -P and -f and no write
permission.


-- 
Regards,
 Mikolaj

Re: ifconfig, wireguard output less verbose, unless -A or

[Mikolaj Kucharski]

Hi,

I tried to address what jmc@ mentioned below. I don't really know
mdoc(7) and English is not my native language, so I imagine there is
place for improvement in the wg(4) diff.

Full diff at the very end of this email, below here `diff -wu` to
visualize how minimal ifconfig.c changes are:


| Index: sbin/ifconfig/ifconfig.c
| ===
| RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
| retrieving revision 1.456
| diff -u -p -w -u -r1.456 ifconfig.c
| --- sbin/ifconfig/ifconfig.c  8 Jul 2022 07:04:54 -   1.456
| +++ sbin/ifconfig/ifconfig.c  3 Sep 2022 08:25:00 -
| @@ -363,7 +363,7 @@ void  unsetwgpeer(const char *, int);
|  void unsetwgpeerpsk(const char *, int);
|  void unsetwgpeerall(const char *, int);
|  
| -void wg_status();
| +void wg_status(int);
|  #else
|  void setignore(const char *, int);
|  #endif
| @@ -679,7 +679,7 @@ void  printgroupattribs(char *);
|  void printif(char *, int);
|  void printb_status(unsigned short, unsigned char *);
|  const char *get_linkstate(int, int);
| -void status(int, struct sockaddr_dl *, int);
| +void status(int, struct sockaddr_dl *, int, int);
|  __dead void  usage(void);
|  const char *get_string(const char *, const char *, u_int8_t *, int *);
|  int  len_string(const u_int8_t *, int);
| @@ -1195,7 +1195,7 @@ printif(char *name, int ifaliases)
|   continue;
|   ifdata = ifa->ifa_data;
|   status(1, (struct sockaddr_dl *)ifa->ifa_addr,
| - ifdata->ifi_link_state);
| + ifdata->ifi_link_state, ifaliases);
|   count++;
|   noinet = 1;
|   continue;
| @@ -3316,7 +3316,7 @@ get_linkstate(int mt, int link_state)
|   * specified, show it and it only; otherwise, show them all.
|   */
|  void
| -status(int link, struct sockaddr_dl *sdl, int ls)
| +status(int link, struct sockaddr_dl *sdl, int ls, int ifaliases)
|  {
|   const struct afswtch *p = afp;
|   struct ifmediareq ifmr;
| @@ -3391,7 +3391,7 @@ status(int link, struct sockaddr_dl *sdl
|   mpls_status();
|   pflow_status();
|   umb_status();
| - wg_status();
| + wg_status(ifaliases);
|  #endif
|   trunk_status();
|   getifgroups();
| @@ -5907,7 +5907,7 @@ process_wg_commands(void)
|  }
|  
|  void
| -wg_status(void)
| +wg_status(int ifaliases)
|  {
|   size_t   i, j, last_size;
|   struct timespec  now;
| @@ -5942,6 +5942,7 @@ wg_status(void)
|   printf("\twgpubkey %s\n", key);
|   }
|  
| + if (ifaliases) {
|   wg_peer = _interface->i_peers[0];
|   for (i = 0; i < wg_interface->i_peers_count; i++) {
|   b64_ntop(wg_peer->p_public, WG_KEY_LEN,
| @@ -5981,6 +5982,7 @@ wg_status(void)
|   wg_aip++;
|   }
|   wg_peer = (struct wg_peer_io *)wg_aip;
| + }
|   }
|  out:
|   free(wgdata.wgd_interface);



On Sat, Aug 20, 2022 at 07:15:39PM +0100, Jason McIntyre wrote:
> On Sat, Aug 20, 2022 at 02:25:25PM +0100, Stuart Henderson wrote:
> > > 
> > > Long output with  as an argument, wgpeers section present:
> > > 
> > > pce-0067# ifconfig.ifaliases wg0
> > > wg0: flags=80c3 mtu 1420
> > > index 8 priority 0 llprio 3
> > > wgport 51820
> > > wgpubkey qcb...
> > > wgpeer klM...
> > > wgpsk (present)
> > > wgpka 25 (sec)
> > > wgendpoint xxx.xxx.xxx.xxx 51820
> > > tx: 178764, rx: 65100
> > > last handshake: 7 seconds ago
> > > wgaip fde4:f456:48c2:13c0::/64
> > > groups: wg
> > > inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64
> > > 
> > > 
> > > Above long output works with group as an argument (ifconfig wg) and with
> > > option -A (ifconfig -A), so I think from user experience perspective,
> > > works as expected.
> > > 
> > > Manual page changes not provided, as I'm not sure are they needed with
> > > this diff.
> > 
> > At least a small change is needed. Maybe some different text would
> > be more appropriate though.
> > 
> > Index: ifconfig.8
> > ===
> > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> > retrieving revision 1.384
> > diff -u -p -r1.384 ifconfig.8
> > --- ifconfig.8  27 Jun 2022 16:27:03 -  1.384
> > +++ ifconfig.8  20 Aug 2022 13:22:43 -
> > @@ -75,7 +75,7 @@ Only the superuser may modify the config
> >  The following options are available:
> >  .Bl -tag -width Ds
> >  .It Fl A
> > -Causes full interface alias information for each interface to
> > +Causes full interface alias and wgpeer information for each interface to
> >  be displayed.
> >  .It Fl a
> >  Causes
> > 
> 
> hi.
> 
> i think this just muddies the simple explanation of -A. i don;t think
> it's worth 

Re: unbound and cannot increase max open fds from 512 to 4152

[Mikolaj Kucharski]

On Fri, Sep 02, 2022 at 11:25:20AM +0200, Sebastian Benoit wrote:
> Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 08:07:01 +:
> > On Fri, Sep 02, 2022 at 09:53:54AM +0200, Sebastian Benoit wrote:
> > > Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 06:47:00 +:
> > > > Hi,
> > > > 
> > > > I have a question, could or should unbound in base be delivered with:
> > > > 
> > > > # cat /etc/login.conf.d/unbound
> > > > unbound:\
> > > > :openfiles-cur=4096:\
> > > > :openfiles-max=8192:\
> > > > :tc=daemon:
> > > > 
> > > > or the like? Above is taken from dovecot. I was able to trigger via
> > > > rcctl reload unbound following warnings in logs:
> > > > 
> > > > Sep  2 06:37:21 x1c unbound: [32940:0] notice: Restart of unbound 
> > > > 1.16.0.
> > > > Sep  2 06:37:21 x1c unbound: [32940:0] warning: setrlimit: Operation 
> > > > not permitted
> > > > Sep  2 06:37:21 x1c unbound: [32940:0] warning: cannot increase max 
> > > > open fds from 512 to 4152
> > > > Sep  2 06:37:21 x1c unbound: [32940:0] warning: continuing with less 
> > > > udp ports: 460
> > > > Sep  2 06:37:21 x1c unbound: [32940:0] warning: increase ulimit or 
> > > > decrease threads, ports in config to remove this warning
> > > > Sep  2 06:37:21 x1c unbound: [32940:0] notice: init module 0: validator
> > > > Sep  2 06:37:21 x1c unbound: [32940:0] notice: init module 1: iterator
> > > > 
> > > > After placing above login.conf.d login class capability file above
> > > > warnings go away:
> > > > 
> > > > Sep  2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound 
> > > > 1.16.0.
> > > > Sep  2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator
> > > > Sep  2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator
> > > 
> > > As far as i understand, the number of fds that unbound is asking for is
> > > based on the num-threads, outgoing-num-tcp, interface and some other 
> > > setting
> > > in the unbound config.
> > 
> > Those particular settings mentioned I did not modified. However the
> > values are not that important. I expected /etc/login.conf.d/unbound to
> > be present on a fresh install. More on that below.
> 
> It would still be interesting why it wants to increase the limit to 4152.
> My list of settings is probably not complete. Show your config if you can.


Sure. Modified a bit to change domain and IP address.

# /var/unbound/etc/unbound.conf
server:
 interface: 127.0.0.1
 interface: ::1

 qname-minimisation: yes

 access-control: 0.0.0.0/0 refuse
 access-control: 127.0.0.0/8 allow
 access-control: ::0/0 refuse
 access-control: ::1 allow

 auto-trust-anchor-file: "/var/unbound/db/root.key"
 val-log-level: 2

 aggressive-nsec: yes

local-zone: "local." static

local-zone: "10.in-addr.arpa." transparent
domain-insecure: "10.in-addr.arpa."

remote-control:
 control-enable: yes
 control-interface: /var/run/unbound.sock

forward-zone:
 name: "example.com."
 forward-addr: 10.123.123.10
 forward-first: no

forward-zone:
 name: "10.in-addr.arpa."
 forward-addr: 10.123.123.10
 forward-first: no

forward-zone:
 name: "."
 forward-addr: 8.8.4.4
 forward-addr: 8.8.8.8
 forward-addr: 208.67.222.222
 forward-addr: 208.67.220.220
 forward-addr: 1.0.0.1
 forward-addr: 1.1.1.1
 forward-first: yes



> Support for login.conf.d was added mostly to support ports that need to make
> modifications to drop files in there.
> 
> I dont expect that the base system will ship with files in login.conf.d
> anytime soon.

Yeah, make sense.


> /B.
> 
> 
> > 
> > 
> > > Did you change any?
> > > 
> > > We already ship with this
> > > 
> > >   unbound:\
> > >   :openfiles=512:\
> > >   :tc=daemon:
> > 
> > Ah, in the main file. I didn't expect this. I started this email thread
> > as I expected daemon which has rc.d script to have separated login class
> > capability file:
> > 
> > # ls -1A /etc/login.conf.d/ | wc -l
> >0
> > 
> > # tar -zvvtf /var/sysmerge/etc.tgz  | grep -c login.conf.d
> > 0
> > 
> > > It is expected that you change login.conf yourself when you tune unbound 
> > > to
> > > your needs.
> > 
> > Yes, I'm aware. That's what I did, but were surprised that there is no
> > pre-existing /etc/login.conf.d/unbound
> > 
> > However now looking again at this, maybe missing /etc/login.conf.d/
> > from base is by design.
> > 

-- 
Regards,
 Mikolaj

Re: unbound and cannot increase max open fds from 512 to 4152

[Mikolaj Kucharski]

On Fri, Sep 02, 2022 at 09:53:54AM +0200, Sebastian Benoit wrote:
> Mikolaj Kucharski(miko...@kucharski.name) on 2022.09.02 06:47:00 +:
> > Hi,
> > 
> > I have a question, could or should unbound in base be delivered with:
> > 
> > # cat /etc/login.conf.d/unbound
> > unbound:\
> > :openfiles-cur=4096:\
> > :openfiles-max=8192:\
> > :tc=daemon:
> > 
> > or the like? Above is taken from dovecot. I was able to trigger via
> > rcctl reload unbound following warnings in logs:
> > 
> > Sep  2 06:37:21 x1c unbound: [32940:0] notice: Restart of unbound 1.16.0.
> > Sep  2 06:37:21 x1c unbound: [32940:0] warning: setrlimit: Operation not 
> > permitted
> > Sep  2 06:37:21 x1c unbound: [32940:0] warning: cannot increase max open 
> > fds from 512 to 4152
> > Sep  2 06:37:21 x1c unbound: [32940:0] warning: continuing with less udp 
> > ports: 460
> > Sep  2 06:37:21 x1c unbound: [32940:0] warning: increase ulimit or decrease 
> > threads, ports in config to remove this warning
> > Sep  2 06:37:21 x1c unbound: [32940:0] notice: init module 0: validator
> > Sep  2 06:37:21 x1c unbound: [32940:0] notice: init module 1: iterator
> > 
> > After placing above login.conf.d login class capability file above
> > warnings go away:
> > 
> > Sep  2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound 1.16.0.
> > Sep  2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator
> > Sep  2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator
> 
> As far as i understand, the number of fds that unbound is asking for is
> based on the num-threads, outgoing-num-tcp, interface and some other setting
> in the unbound config.

Those particular settings mentioned I did not modified. However the
values are not that important. I expected /etc/login.conf.d/unbound to
be present on a fresh install. More on that below.


> Did you change any?
> 
> We already ship with this
> 
>   unbound:\
>   :openfiles=512:\
>   :tc=daemon:

Ah, in the main file. I didn't expect this. I started this email thread
as I expected daemon which has rc.d script to have separated login class
capability file:

# ls -1A /etc/login.conf.d/ | wc -l
   0

# tar -zvvtf /var/sysmerge/etc.tgz  | grep -c login.conf.d
0

> It is expected that you change login.conf yourself when you tune unbound to
> your needs.

Yes, I'm aware. That's what I did, but were surprised that there is no
pre-existing /etc/login.conf.d/unbound

However now looking again at this, maybe missing /etc/login.conf.d/
from base is by design.

-- 
Regards,
 Mikolaj

unbound and cannot increase max open fds from 512 to 4152

[Mikolaj Kucharski]

Hi,

I have a question, could or should unbound in base be delivered with:

# cat /etc/login.conf.d/unbound
unbound:\
:openfiles-cur=4096:\
:openfiles-max=8192:\
:tc=daemon:

or the like? Above is taken from dovecot. I was able to trigger via
rcctl reload unbound following warnings in logs:

Sep  2 06:37:21 x1c unbound: [32940:0] notice: Restart of unbound 1.16.0.
Sep  2 06:37:21 x1c unbound: [32940:0] warning: setrlimit: Operation not 
permitted
Sep  2 06:37:21 x1c unbound: [32940:0] warning: cannot increase max open fds 
from 512 to 4152
Sep  2 06:37:21 x1c unbound: [32940:0] warning: continuing with less udp ports: 
460
Sep  2 06:37:21 x1c unbound: [32940:0] warning: increase ulimit or decrease 
threads, ports in config to remove this warning
Sep  2 06:37:21 x1c unbound: [32940:0] notice: init module 0: validator
Sep  2 06:37:21 x1c unbound: [32940:0] notice: init module 1: iterator

After placing above login.conf.d login class capability file above
warnings go away:

Sep  2 06:39:58 x1c unbound: [14264:0] notice: Restart of unbound 1.16.0.
Sep  2 06:39:58 x1c unbound: [14264:0] notice: init module 0: validator
Sep  2 06:39:58 x1c unbound: [14264:0] notice: init module 1: iterator


-- 
Regards,
 Mikolaj

ifconfig, wireguard output less verbose, unless -A or

[Mikolaj Kucharski]

Hi,

Per other thread, Theo expressed dissatisfaction with long ifconfig(8)
for wg(4) interface. Stuart Henderson pointed me at direction, which
below diff makes it work.

I guess to questions are:

- Does the behaviour of ifconfig(8) make sense?
- Does the code which makes above, make sense?

This is minimal diff, I would appreciate feedback, I did least
resistance approach. Looking at diff -wu shows even less changes
as wg_status() is mainly identation with if-statement.


Short output by default, only 6 lines, no wgpeers section:

pce-0067# ifconfig.ifaliases -a | tail -n6
wg0: flags=80c3 mtu 1420
index 8 priority 0 llprio 3
wgport 51820
wgpubkey qcb...
groups: wg
inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64


Long output with  as an argument, wgpeers section present:

pce-0067# ifconfig.ifaliases wg0
wg0: flags=80c3 mtu 1420
index 8 priority 0 llprio 3
wgport 51820
wgpubkey qcb...
wgpeer klM...
wgpsk (present)
wgpka 25 (sec)
wgendpoint xxx.xxx.xxx.xxx 51820
tx: 178764, rx: 65100
last handshake: 7 seconds ago
wgaip fde4:f456:48c2:13c0::/64
groups: wg
inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64


Above long output works with group as an argument (ifconfig wg) and with
option -A (ifconfig -A), so I think from user experience perspective,
works as expected.

Manual page changes not provided, as I'm not sure are they needed with
this diff.

Comments?


Index: ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.456
diff -u -p -u -r1.456 ifconfig.c
--- ifconfig.c  8 Jul 2022 07:04:54 -   1.456
+++ ifconfig.c  14 Jul 2022 09:25:21 -
@@ -363,7 +363,7 @@ voidunsetwgpeer(const char *, int);
 void   unsetwgpeerpsk(const char *, int);
 void   unsetwgpeerall(const char *, int);
 
-void   wg_status();
+void   wg_status(int);
 #else
 void   setignore(const char *, int);
 #endif
@@ -679,7 +679,7 @@ voidprintgroupattribs(char *);
 void   printif(char *, int);
 void   printb_status(unsigned short, unsigned char *);
 const char *get_linkstate(int, int);
-void   status(int, struct sockaddr_dl *, int);
+void   status(int, struct sockaddr_dl *, int, int);
 __dead voidusage(void);
 const char *get_string(const char *, const char *, u_int8_t *, int *);
 intlen_string(const u_int8_t *, int);
@@ -1195,7 +1195,7 @@ printif(char *name, int ifaliases)
continue;
ifdata = ifa->ifa_data;
status(1, (struct sockaddr_dl *)ifa->ifa_addr,
-   ifdata->ifi_link_state);
+   ifdata->ifi_link_state, ifaliases);
count++;
noinet = 1;
continue;
@@ -3316,7 +3316,7 @@ get_linkstate(int mt, int link_state)
  * specified, show it and it only; otherwise, show them all.
  */
 void
-status(int link, struct sockaddr_dl *sdl, int ls)
+status(int link, struct sockaddr_dl *sdl, int ls, int ifaliases)
 {
const struct afswtch *p = afp;
struct ifmediareq ifmr;
@@ -3391,7 +3391,7 @@ status(int link, struct sockaddr_dl *sdl
mpls_status();
pflow_status();
umb_status();
-   wg_status();
+   wg_status(ifaliases);
 #endif
trunk_status();
getifgroups();
@@ -5907,7 +5907,7 @@ process_wg_commands(void)
 }
 
 void
-wg_status(void)
+wg_status(int ifaliases)
 {
size_t   i, j, last_size;
struct timespec  now;
@@ -5942,45 +5942,47 @@ wg_status(void)
printf("\twgpubkey %s\n", key);
}
 
-   wg_peer = _interface->i_peers[0];
-   for (i = 0; i < wg_interface->i_peers_count; i++) {
-   b64_ntop(wg_peer->p_public, WG_KEY_LEN,
-   key, sizeof(key));
-   printf("\twgpeer %s\n", key);
-
-   if (wg_peer->p_flags & WG_PEER_HAS_PSK)
-   printf("\t\twgpsk (present)\n");
-
-   if (wg_peer->p_flags & WG_PEER_HAS_PKA && wg_peer->p_pka)
-   printf("\t\twgpka %u (sec)\n", wg_peer->p_pka);
-
-   if (wg_peer->p_flags & WG_PEER_HAS_ENDPOINT) {
-   if (getnameinfo(_peer->p_sa, wg_peer->p_sa.sa_len,
-   hbuf, sizeof(hbuf), sbuf, sizeof(sbuf),
-   NI_NUMERICHOST | NI_NUMERICSERV) == 0)
-   printf("\t\twgendpoint %s %s\n", hbuf, sbuf);
-   else
-   printf("\t\twgendpoint unable to print\n");
-   }
+   if (ifaliases) {
+   wg_peer = _interface->i_peers[0];
+   for (i = 0; i < wg_interface->i_peers_count; i++) {
+   b64_ntop(wg_peer->p_public, 

Re: cwm: remove menu-ssh

[Mikolaj Kucharski]

I find the feature very useful. It's one of the most used menus in cwm,
next to exec. Does that feature really need to go?

-- 
Regards,
 Mikolaj

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

On Wed, Jul 13, 2022 at 05:43:59PM +0100, Stuart Henderson wrote:
> > 
> > Not sure how to handle long output in different way. If you don't
> > specify wgdesc to the ifconfig, the diff doesn't change anything and
> > ifconfig(8) output is exactly the same. If you don't find this feature
> > useful, by not using it, nothing changes for you. Isn't that fair?
> 
> It might be better if wgpeer details would be skipped from plain
> "ifconfig" and only listed if "ifconfig wg0" is used, much like
> IPv4 aliases are skipped from "ifconfig" and only listed when
> you use either "ifconfig -A" or "ifconfig em0" etc.
> 

Ah, that helps. Thank you! Will look into it. Indeed this makes more
sense. I was aware of -A, but was not aware of ifconfig  behaviour.

-- 
Regards,
 Mikolaj

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

On Wed, Jul 13, 2022 at 10:02:30AM -0600, Theo de Raadt wrote:
> Mikolaj Kucharski  wrote:
> 
> > I took the libery and refreshed the patch. What I did so far:
> > 
> > - compiled GENERIC.MP on amd64
> > - compiled new ifconfig, same arch
> > - booted up new bsd.mp with the patch
> > - when wgdesc is not set, pre-patch ifconfig seems to work
> > - when with new ifconfig I set wgdesc, old ifconfig wg segfaults
> > 
> > Example from running -current:
> > 
> > pce-0067# ifconfig.new wg0 
> > wg0: flags=80c3 mtu 1420
> > index 8 priority 0 llprio 3
> > wgport 51820
> > wgpubkey qcb...
> > wgpeer klM...
> > description: ks2
> > wgpsk (present)
> > wgpka 25 (sec)
> > wgendpoint xxx.xxx.xxx.xxx 51820
> > tx: 1932, rx: 620
> > last handshake: 83 seconds ago
> > wgaip fde4:f456:48c2:13c0::/64
> > groups: wg
> > inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64
> 
> That seems disgustingly verbose to me.
> 
> Who is going to read it?

Me. I find this one additional line useful.

> Shall we put all the active counters for normal ethernet/wifi into the
> default ifconfig output, so that the default ifconfig output scrolls and
> scrolls and scrolls and noone actually reads it?
> 
> I think this has gone off the rails.

This is the nature of wg(4) interface. I have machine with 25 peers and
each peer adds lines to the ifconfig(8) output. This is on a machine
without patch from this thread, -stable, official kernel:

# ifconfig wg0 | wc -l
 128

Not sure how to handle long output in different way. If you don't
specify wgdesc to the ifconfig, the diff doesn't change anything and
ifconfig(8) output is exactly the same. If you don't find this feature
useful, by not using it, nothing changes for you. Isn't that fair?

-- 
Regards,
 Mikolaj

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

On Wed, Jul 13, 2022 at 03:37:05PM +, Mikolaj Kucharski wrote:
> Example from running -current:
> 
> pce-0067# ifconfig.new wg0 
> wg0: flags=80c3 mtu 1420
> index 8 priority 0 llprio 3
> wgport 51820
> wgpubkey qcb...
> wgpeer klM...
> description: ks2
> wgpsk (present)
> wgpka 25 (sec)
> wgendpoint xxx.xxx.xxx.xxx 51820
> tx: 1932, rx: 620
> last handshake: 83 seconds ago
> wgaip fde4:f456:48c2:13c0::/64
> groups: wg
> inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64
> 
> Seems to work for me. Looking at above ifconfig(8) output, not sure
> should the output should say `description:` or just follow the pattern
> of other wireguard keywords and just say `wgdesc` (same keyword like
> for ifconfig command and no colon). I think I would prefer that.
> 

I mean like this:

--- ifconfig.c  Wed Jul 13 15:19:24 2022
+++ ifconfig.c  Wed Jul 13 15:39:04 2022
@@ -5972,7 +5972,7 @@
printf("\twgpeer %s\n", key);
 
if (strlen(wg_peer->p_description))
-   printf("\t\tdescription: %s\n", wg_peer->p_description);
+   printf("\t\twgdesc %s\n", wg_peer->p_description);
 
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");



pce-0067# ifconfig.new wg
wg0: flags=80c3 mtu 1420
index 8 priority 0 llprio 3
wgport 51820
wgpubkey qcb...
wgpeer klM...
wgdesc ks2
wgpsk (present)
wgpka 25 (sec)
wgendpoint xxx.xxx.xxx.xxx 51820
tx: 3260, rx: 1116
last handshake: 69 seconds ago
wgaip fde4:f456:48c2:13c0::/64
groups: wg
inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64

-- 
Regards,
 Mikolaj

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

I took the libery and refreshed the patch. What I did so far:

- compiled GENERIC.MP on amd64
- compiled new ifconfig, same arch
- booted up new bsd.mp with the patch
- when wgdesc is not set, pre-patch ifconfig seems to work
- when with new ifconfig I set wgdesc, old ifconfig wg segfaults

Example from running -current:

pce-0067# ifconfig.new wg0 
wg0: flags=80c3 mtu 1420
index 8 priority 0 llprio 3
wgport 51820
wgpubkey qcb...
wgpeer klM...
description: ks2
wgpsk (present)
wgpka 25 (sec)
wgendpoint xxx.xxx.xxx.xxx 51820
tx: 1932, rx: 620
last handshake: 83 seconds ago
wgaip fde4:f456:48c2:13c0::/64
groups: wg
inet6 fde4:f456:48c2:13c0::cc67 prefixlen 64

Seems to work for me. Looking at above ifconfig(8) output, not sure
should the output should say `description:` or just follow the pattern
of other wireguard keywords and just say `wgdesc` (same keyword like
for ifconfig command and no colon). I think I would prefer that.

Patch is not mine. It's from Noah Meier  and
I just re-applied it to -current. It's at the end of this email.

I hope I didn't screw up anything.


On Wed, Jul 13, 2022 at 11:29:51AM +, Mikolaj Kucharski wrote:
> Hi,
> 
> I'm sorry I didn't test this, as I don't have -current OpenBSD on all my
> machines, but I do have one WireGuard gateway running -stable with few
> peers:
> 
> # ifconfig wg | grep -cw wgpeer
> 25
> 
> I would love to have this merged into main repo, as it would make my
> life a tiny bit easier to see which pubkey is which peer.
> 
> Based on what Stefan wrote below, do you have by any chance newer
> version of your diff, Noah?
> 
> 
> 
> On Wed, Dec 01, 2021 at 12:18:52AM +0100, Stefan Sperling wrote:
> > On Tue, Nov 30, 2021 at 02:31:20PM -0500, Noah Meier wrote:
> > > Hi Stefan,
> > > 
> > > Richard Procter offered some kind advice on the ordering of options in 
> > > the man page
> > > (to be done alphabetically) and commented on an unnecessary cast.
> > > 
> > > I also believe that I goofed by failing to initalize the mutex and zero 
> > > the description
> > > upon peer creation (in wg_peer_create).
> > > 
> > > I’ve attempted to address these issues and have pasted the diff below.
> > > 
> > > NM
> > 
> > This new patch does not apply cleanly.
> > Leading whitespace was stripped, and thus patch complains as follows:
> > 
> > Patching file if_wg.c using Plan A...
> > patch:  malformed patch at line 15: };
> > 
> > And it would help if you created a patch where all paths are relative to
> > the /usr/src directory. Something like this should do it:
> > 
> >  cd /usr/src
> >  cvs diff -u sbin/ifconfig sys/net  > /tmp/wgdesc.patch
> > 
> > If your mailer cannot preserve whitespace as-is then please try attaching
> > the patch file instead of inlining it.
> > 
> > Thanks!




Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.384
diff -u -p -u -r1.384 ifconfig.8
--- sbin/ifconfig/ifconfig.827 Jun 2022 16:27:03 -  1.384
+++ sbin/ifconfig/ifconfig.813 Jul 2022 14:53:22 -
@@ -2251,6 +2251,10 @@ Set the peer's IPv4 or IPv6
 range for tunneled traffic.
 Repeat the option to set multiple ranges.
 By default, no addresses are allowed.
+.It Cm wgdesc Ar value
+Specify a description of the peer.
+.It Cm -wgdesc
+Clear the peer description.
 .It Cm wgendpoint Ar peer_address port
 Address traffic to the peer's IPv4 or IPv6
 .Ar peer_address
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.456
diff -u -p -u -r1.456 ifconfig.c
--- sbin/ifconfig/ifconfig.c8 Jul 2022 07:04:54 -   1.456
+++ sbin/ifconfig/ifconfig.c13 Jul 2022 14:53:22 -
@@ -355,12 +355,14 @@ void  setwgpeerep(const char *, const cha
 void   setwgpeeraip(const char *, int);
 void   setwgpeerpsk(const char *, int);
 void   setwgpeerpka(const char *, int);
+void   setwgpeerdesc(const char *, int);
 void   setwgport(const char *, int);
 void   setwgkey(const char *, int);
 void   setwgrtable(const char *, int);
 
 void   unsetwgpeer(const char *, int);
 void   unsetwgpeerpsk(const char *, int);
+void   unsetwgpeerdesc(const char *, int);
 void   unsetwgpeerall(const char *, int);
 
 void   wg_status();
@@ -620,11 +622,13 @@ const struct  cmd {
{ "wgaip",  NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk",  NEXTARG,A_WI

Re: ifconfig description for wireguard peers

[Mikolaj Kucharski]

Hi,

I'm sorry I didn't test this, as I don't have -current OpenBSD on all my
machines, but I do have one WireGuard gateway running -stable with few
peers:

# ifconfig wg | grep -cw wgpeer
25

I would love to have this merged into main repo, as it would make my
life a tiny bit easier to see which pubkey is which peer.

Based on what Stefan wrote below, do you have by any chance newer
version of your diff, Noah?



On Wed, Dec 01, 2021 at 12:18:52AM +0100, Stefan Sperling wrote:
> On Tue, Nov 30, 2021 at 02:31:20PM -0500, Noah Meier wrote:
> > Hi Stefan,
> > 
> > Richard Procter offered some kind advice on the ordering of options in the 
> > man page
> > (to be done alphabetically) and commented on an unnecessary cast.
> > 
> > I also believe that I goofed by failing to initalize the mutex and zero the 
> > description
> > upon peer creation (in wg_peer_create).
> > 
> > I’ve attempted to address these issues and have pasted the diff below.
> > 
> > NM
> 
> This new patch does not apply cleanly.
> Leading whitespace was stripped, and thus patch complains as follows:
> 
> Patching file if_wg.c using Plan A...
> patch:  malformed patch at line 15: };
> 
> And it would help if you created a patch where all paths are relative to
> the /usr/src directory. Something like this should do it:
> 
>  cd /usr/src
>  cvs diff -u sbin/ifconfig sys/net  > /tmp/wgdesc.patch
> 
> If your mailer cannot preserve whitespace as-is then please try attaching
> the patch file instead of inlining it.
> 
> Thanks!
> 

-- 
Regards,
 Mikolaj

Re: close net80211 hardware crypto set_key races

[Mikolaj Kucharski]

Hi Stefan,

I'm not yet ready to test new diffs, but I was going through old
wireless stack related emails and was wondering did below patch got
committed? Per my git / cvs search I don't think so. Is below diff
still relevant or can below change be ignored?

On Thu, Jul 01, 2021 at 12:23:33PM +0200, Stefan Sperling wrote:
> Some wifi drivers defer installation of keys into a process context. 
> There was a noticable race in the past which was fixed thanks to krw@
> where link state was set to UP before key installation had completed in
> hardware. This race could result in dhclient failing to get a lease.
> 
> Other races remain. These concern state which controls encryption of
> frames in the output path of net80211 (TXRXPROT flag), and marks the
> pairwise key as installed (PTKDONE state) which in turn affects our
> acceptance of group key updates sent by the AP. While either of these
> flags is set and hardware is not configured for encryption we are running
> in an inconsistent state. I don't know if this causes problems in practice
> but we should be avoiding such races. The fix is fairly mechanical and
> essentially defers modification of relevant flags from net80211 into the
> affected drivers.
> 
> Affected drivers are: iwx, bfwm, athn (USB only!), urtwn, run, rsu, otus.
> I have tested on iwx, athn, urtwn, and run. More tests are welcome.
> 
> ok?
> 
> diff 86b3b873864f7e98e688c12d8e455803ff30eadb 
> 27f93922f2b250b31e9534b457665df3dcf58794
> blob - ea245574f624ada1b8d0c9e8e8a0653fb2c3adc4
> blob + 0bac2cda2290c4a44de8f5b8110a4399f75de88d
> --- sys/dev/ic/bwfm.c
> +++ sys/dev/ic/bwfm.c
> @@ -2714,7 +2714,13 @@ bwfm_set_key_cb(struct bwfm_softc *sc, void *arg)
>   wsec |= wsec_enable;
>   bwfm_fwvar_var_set_int(sc, "wsec", wsec);
>  
> + if ((k->k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> +
>   if (sc->sc_key_tasks == 0) {
> + if (k->k_flags & IEEE80211_KEY_SECURE)
> + ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
>   DPRINTF(("%s: marking port %s valid\n", DEVNAME(sc),
>   ether_sprintf(cmd->ni->ni_macaddr)));
>   cmd->ni->ni_port_valid = 1;
> blob - bdf8ce3e1afa332f698e3dc56af77e6acb4f8689
> blob + 5be1d92ee862f584df5901fe54ebe30e0c937d36
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -6747,12 +6747,17 @@ iwx_add_sta_key(struct iwx_softc *sc, int sta_id, stru
>   return err;
>   }
>  
> - if (k->k_flags & IEEE80211_KEY_GROUP)
> + if (k->k_flags & IEEE80211_KEY_GROUP) {
>   in->in_flags |= IWX_NODE_FLAG_HAVE_GROUP_KEY;
> - else
> + } else {
>   in->in_flags |= IWX_NODE_FLAG_HAVE_PAIRWISE_KEY;
> + if (ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
> + }
>  
>   if ((in->in_flags & want_keymask) == want_keymask) {
> + if (k->k_flags & IEEE80211_KEY_SECURE)
> + ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
>   DPRINTF(("marking port %s valid\n",
>   ether_sprintf(ni->ni_macaddr)));
>   ni->ni_port_valid = 1;
> blob - 08ffda8c9aaede9c901d97a43bf2873591e8df19
> blob + 0c141fc5df7ba1a774af98a59a2b718bdcc721bb
> --- sys/dev/usb/if_athn_usb.c
> +++ sys/dev/usb/if_athn_usb.c
> @@ -1662,7 +1662,12 @@ athn_usb_set_key_cb(struct athn_usb_softc *usc, void *
>   s = splnet();
>   athn_usb_write_barrier(>sc_sc);
>   athn_set_key(ic, cmd->ni, cmd->key);
> + if ((cmd->key->k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + cmd->ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + cmd->ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
>   if (usc->sc_key_tasks == 0) {
> + if (cmd->key->k_flags & IEEE80211_KEY_SECURE)
> + cmd->ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
>   DPRINTF(("marking port %s valid\n",
>   ether_sprintf(cmd->ni->ni_macaddr)));
>   cmd->ni->ni_port_valid = 1;
> blob - bb220831ffc852347afdcda18c9bca0589d6d939
> blob + 8e426e9cab0b389f2e1260139ad451b30e10287e
> --- sys/dev/usb/if_otus.c
> +++ sys/dev/usb/if_otus.c
> @@ -2116,7 +2116,12 @@ otus_set_key_cb(struct otus_softc *sc, void *arg)
>   memcpy(key.key, k->k_key + 16, 16);
>   (void)otus_cmd(sc, AR_CMD_EKEY, , sizeof key, NULL);
>  
> + if ((k->k_flags & IEEE80211_KEY_GROUP) == 0 &&
> + cmd->ni->ni_rsn_supp_state == RSNA_SUPP_PTKNEGOTIATING)
> + cmd->ni->ni_rsn_supp_state = RSNA_SUPP_PTKDONE;
>   if (sc->sc_key_tasks == 0) {
> + if (k->k_flags & IEEE80211_KEY_SECURE)
> + cmd->ni->ni_flags |= IEEE80211_NODE_TXRXPROT;
>   DPRINTF(("marking port %s valid\n",
>   ether_sprintf(cmd->ni->ni_macaddr)));
>   

Re: sppp(4)/pppoe(4) - avoid endless loop in remote ip negotiation

[Mikolaj Kucharski]

On Fri, Nov 26, 2021 at 01:35:14PM +0100, Krzysztof Kanas wrote:
> Hi. When remote side in sppp doesn't reply for to PPP IPCP IP-Address 
> sppp will try to negotiate remote IP in endless loop. Instead use 
> 10.64.64.1 + if_index as remote IP.
> 

Okay, but why it doesn't reply? I don't use anymore PPPoE, as none
of the machines connected to the internet use that mechanism, but
from my very old backups I see I used in the past:


# /etc/hostname.pppoe0
inet 0.0.0.0 255.255.255.255 NONE pppoedev vr0 authproto chap \
authname 'some-customer-id-here' authkey 'secret-key' \
description "pppoe uplink" up
dest 0.0.0.1
!route -nq add default -ifp \$if 0.0.0.1


I don't remember any issues with above setup and that OpenBSD
machine was running -current and was rebooted many times during
its 6+ years of service. Also machine faced many power outages
and on a fresh bootup I don't remember it not able to connect,
that I needed to run a custom kernel or do some extensive
troubleshooting.

Looking again at pppoe(4) manual page I copied above config
directly from there.

What problem are you trying to solve here is not clear to me.

-- 
Regards,
 Mikolaj

Re: spamd(8) use tls_config_set_{cert,key}_file instead of relying on tls_load_file(3)

[Mikolaj Kucharski]

On Tue, Jul 06, 2021 at 12:58:37PM +, Klemens Nanni wrote:
> On Wed, Jun 30, 2021 at 01:11:38PM +0100, Ricardo Mestre wrote:
> > Hi,
> > 
> > I may have seen it elsewhere, or probably not, but after checking on kn's 
> > commit
> > to tls_load_file(3) it seems it's now possible to set the ca/cert/key 
> > directly
> > without having to load them first from disk and set them afterwards from 
> > memory.
> 
> That's correct and the tls_config_set_*_{mem -> file}() changes are
> correct.
> 
> > That being said the below applies this to spamd(8), no functional change 
> > apart
> > from setting up TLS upfront, and now also before pledge(2). I tested it on 
> > one
> > of my servers without issues, but also with only the cert or just with the 
> > key
> > to ensure it would still error out.
> 
> I am not a spamd user but this is what I see from code inspection:
> Currently, things happen in this order of
> 
>   tls_load_file()
>   fork()
>   setres*id()
>   pledge()
>   tls_config_set_*_mem()
> 
> and your diff looks like changing that to
> 
>   fork()
>   setres*id()
>   tls_config_set_*_file()
>   pledge()
> 
> So it seems that currently file permissions on both TLS certificate and
> privat key files don't matter as they're read as root, while with your
> diff they'd be read as the unprivileged _spamd user?
> 
> I didn't find details about required permissions for those files in our
> manual pages, but I'd expect them to be `root:wheel 0700' or so, i.e.
> not readable by anyone but root.
> 
> Am I missing something here or are your TLS files readable by _spamd so
> that the diff works (for you)?

I can confirm that my 6.9-stable running spamd(8) for couple of years
now has following:

# rcctl get spamd
spamd_class=daemon
spamd_flags=-C /etc/mail/certs/example.com-full.crt -K 
/etc/mail/certs/example.com.key -h example.com -v
spamd_logger=
spamd_rtable=0
spamd_timeout=30
spamd_user=root

# ls -l /etc/mail/certs/example.com-full.crt /etc/mail/certs/example.com.key
-r--r--r--  1 root  wheel  3778 May  2 20:04 
/etc/mail/certs/example.com-full.crt
-r  1 root  wheel  3272 Sep 16  2019 /etc/mail/certs/example.com.key

 
> > Index: spamd.c
> > ===
> > RCS file: /cvs/src/libexec/spamd/spamd.c,v
> > retrieving revision 1.156
> > diff -u -p -u -r1.156 spamd.c
> > --- spamd.c 6 Aug 2019 13:34:36 -   1.156
> > +++ spamd.c 30 Jun 2021 11:53:18 -
> > @@ -136,10 +136,6 @@ u_short cfg_port;
> >  u_short sync_port;
> >  struct tls_config *tlscfg;
> >  struct tls *tlsctx;
> > -uint8_t*pubcert;
> > -size_t  pubcertlen;
> > -uint8_t*privkey;
> > -size_t  privkeylen;
> >  char   *tlskeyfile = NULL;
> >  char   *tlscertfile = NULL;
> >  
> > @@ -464,9 +460,9 @@ spamd_tls_init()
> > if (tls_config_set_ciphers(tlscfg, "all") != 0)
> > errx(1, "failed to set tls ciphers");
> >  
> > -   if (tls_config_set_cert_mem(tlscfg, pubcert, pubcertlen) == -1)
> > +   if (tls_config_set_cert_file(tlscfg, tlscertfile) == -1)
> > errx(1, "unable to set TLS certificate file %s", tlscertfile);
> > -   if (tls_config_set_key_mem(tlscfg, privkey, privkeylen) == -1)
> > +   if (tls_config_set_key_file(tlscfg, tlskeyfile) == -1)
> > errx(1, "unable to set TLS key file %s", tlskeyfile);
> > if (tls_configure(tlsctx, tlscfg) != 0)
> > errx(1, "failed to configure TLS - %s", tls_error(tlsctx));
> > @@ -1392,12 +1388,7 @@ main(int argc, char *argv[])
> > } else if (maxblack > maxcon)
> > usage();
> >  
> > -   if (tlscertfile &&
> > -   (pubcert=tls_load_file(tlscertfile, , NULL)) == NULL)
> > -   errx(1, "unable to load TLS certificate file %s", tlscertfile);
> > -   if (tlskeyfile &&
> > -   (privkey=tls_load_file(tlskeyfile, , NULL)) == NULL)
> > -   errx(1, "unable to load TLS key file %s", tlskeyfile);
> > +   spamd_tls_init();
> >  
> > rlp.rlim_cur = rlp.rlim_max = maxcon + 15;
> > if (setrlimit(RLIMIT_NOFILE, ) == -1)
> > @@ -1546,8 +1537,6 @@ main(int argc, char *argv[])
> >  jail:
> > if (pledge("stdio inet", NULL) == -1)
> > err(1, "pledge");
> > -
> > -   spamd_tls_init();
> >  
> > if (listen(smtplisten, 10) == -1)
> > err(1, "listen");
> > 
> 

-- 
Regards,
 Mikolaj

Re: athn(4): fix corrupt input frames

[Mikolaj Kucharski]

On Tue, May 11, 2021 at 04:12:34PM +0200, Stefan Sperling wrote:
> kettenis@ noticed that an athn(4) node cache can contain bogus clients
> with MAC addreses that look like bit-flipped versions of MAC addresses
> of legitimate clients.
> 
> I think this is due to my recent fix to allow block ack requests to
> pass through athn(4).
> 
> The driver calls ieee80211_find_rxnode() on such frames.
> In hostap mode this function is responsible for creating new node cache
> entries in case the transmitter's address is not known yet. This results
> in bogus cache entries. This patch attempts to fix that issue by searching
> the transmitter address in the cache and dropping the frame if the address
> is unknown.
> 
> In client mode ieee80211_find_rxnode() always returns ic->ic_bss.
> We can simply validate the transmitter address in the frame against
> the AP's MAC adress which is always known.
> 
> In monitor mode we can pass such frames since they will be dropped
> anyway after bpf_mtap.
> 
> Even if the address is correct other parts of the frame might still be
> corrupt. I don't think there is a way to verify the frame's checksum if
> the hardware cannot do it for us.
> I hope that net80211's input path is robust enough to deal with this...
> 
> ok?

Currently testing on:

// Wi-Fi client to pce-0041
pce-0067# dmesg | grep athn
athn0 at pci4 dev 0 function 0 "Atheros AR9281" rev 0x01: apic 5 int 16
athn0: AR9280 rev 2 (2T2R), ROM rev 22, address 04:f0:21:45:6a:c2

// accesspoint
pce-0041# dmesg | grep athn
athn0 at pci4 dev 0 function 0 "Atheros AR9281" rev 0x01: apic 5 int 16
athn0: AR9280 rev 2 (2T2R), ROM rev 22, address 04:f0:21:34:e4:23

// accesspoint
pce-0035# dmesg | grep athn
athn0 at pci4 dev 0 function 0 "Atheros AR9281" rev 0x01: apic 5 int 16
athn0: AR9280 rev 2 (2T2R), ROM rev 22, address 04:f0:21:45:6a:c4

At the time of this mail, they have about 20+ minutes of uptime. No
noticable issues so far.


> diff 61810dc72eb09d7c410d5b2c7982a940951e4dd4 /usr/src
> blob - 816a3b09ee2f59b2a22c4a2d236e39272b10
> file + sys/dev/ic/ar5008.c
> --- sys/dev/ic/ar5008.c
> +++ sys/dev/ic/ar5008.c
> @@ -970,29 +970,52 @@ ar5008_rx_process(struct athn_softc *sc, struct mbuf_l
>   /* Finalize mbuf. */
>   m->m_pkthdr.len = m->m_len = len;
>  
> - /* Grab a reference to the source node. */
>   wh = mtod(m, struct ieee80211_frame *);
> - ni = ieee80211_find_rxnode(ic, wh);
>  
>   if (michael_mic_failure) {
>   /*
>* Check that it is not a control frame
>* (invalid MIC failures on valid ctl frames).
> +  * Validate the transmitter's address to avoid passing
> +  * corrupt frames with bogus addresses to net80211.
>*/
> - if (!(wh->i_fc[0] & IEEE80211_FC0_TYPE_CTL) &&
> - (ic->ic_flags & IEEE80211_F_RSNON) &&
> - (ni->ni_rsncipher == IEEE80211_CIPHER_TKIP ||
> - ni->ni_rsngroupcipher == IEEE80211_CIPHER_TKIP)) {
> - /* Report Michael MIC failures to net80211. */
> - ic->ic_stats.is_rx_locmicfail++;
> - ieee80211_michael_mic_failure(ic, 0);
> + if ((wh->i_fc[0] & IEEE80211_FC0_TYPE_CTL)) {
> + switch (ic->ic_opmode) {
> +#ifndef IEEE80211_STA_ONLY
> + case IEEE80211_M_HOSTAP:
> + if (ieee80211_find_node(ic, wh->i_addr2))
> + michael_mic_failure = 0;
> + break;
> +#endif
> + case IEEE80211_M_STA:
> + if (IEEE80211_ADDR_EQ(wh->i_addr2,
> + ic->ic_bss->ni_macaddr))
> + michael_mic_failure = 0;
> + break;
> + case IEEE80211_M_MONITOR:
> + michael_mic_failure = 0;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (michael_mic_failure) {
> + /* Report Michael MIC failures to net80211. */
> + if ((ic->ic_rsnciphers & IEEE80211_CIPHER_TKIP) ||
> + ic->ic_rsngroupcipher == IEEE80211_CIPHER_TKIP) {
> + ic->ic_stats.is_rx_locmicfail++;
> + ieee80211_michael_mic_failure(ic, 0);
> + }
>   ifp->if_ierrors++;
> - ieee80211_release_node(ic, ni);
>   m_freem(m);
>   goto skip;
>   }
>   }
>  
> + /* Grab a reference to the source node. */
> + ni = ieee80211_find_rxnode(ic, wh);
> +
>   /* Remove any HW padding after the 802.11 header. */
>   if (!(wh->i_fc[0] & IEEE80211_FC0_TYPE_CTL)) {
>   

Re: athn(4): switch Tx rate control to RA

[Mikolaj Kucharski]

On Wed, Mar 31, 2021 at 11:31:19AM +, Mikolaj Kucharski wrote:
> On Wed, Mar 31, 2021 at 11:24:19AM +0000, Mikolaj Kucharski wrote:
> > On Tue, Mar 23, 2021 at 07:47:08PM +, Mikolaj Kucharski wrote:
> > > On Tue, Mar 23, 2021 at 07:06:33PM +, Mikolaj Kucharski wrote:
> > > > On Tue, Mar 23, 2021 at 06:01:27PM +0100, Stefan Sperling wrote:
> > > > > This switches athn(4) to the new RA Tx rate adaptation module.
> > > > > Tests on athn(4) PCI devices are welcome.
> > > > > USB devices don't need to be tested in this case Tx rate adaptation
> > > > > is taken care of by firmware.
> > > > > 
> > > > > I could only test on AR9285 so far, but the result looks promising.
> > > > > 
> > > > > diff c6a6a64b49f2287751632205d64f594eb6c1ee42 
> > > > > 636e9964c6e5313bb1c75823249d483597a0e93a
> > > > ...
> > > > 
> > > > 
> > > > Tested on athn(4) in `mediaopt hostap` mode. Uptime 1hr, so not a lot of
> > > > testing yet. So far no issues. I have two systems like that (in hostap)
> > > > and in `dmesg | grep athn` only difference is mac address between them.
> > > > I can send full dmesg from second system as well if you want.
> > > > 
> > > 
> > > I also have third system, with the same athn(4) card (only mac address
> > > is different in `dmesg | grep athn` output), but it acts as a Wi-Fi
> > > client and is connected to OpenBSD athn(4)-based access point from
> > > my previous email (full dmesg output in an earlier email).
> > > 
> > 
> > After week of running this I have similar observations like Scott
> > Bennett.
> > 
> > I will focus on one location, as I have 2 access points running on
> > athn(4) with the diff from this thread, but I'm only present at one of
> > those locations. All athn(4) machines have the diff applied.
> > 
> > OpenBSD, athn(4), hostap
> > OpenBSD, athn(4), wi-fi client to above access point
> > OpenBSD, iwm(4), wi-fi client to above access point
> > 
> > I do see some packets dropped with RA patch:
> > 
> > # mtr -rwb -c 1000 192.168.220.76
> > Start: 2021-03-31T08:17:57+
> > HOST: pce-0041.home.lan Loss%   Snt   Last   Avg  Best  Wrst StDev
> >   1.|-- 192.168.220.76 9.2%  10002.2   2.6   1.2  82.9   3.4
> > 
> > Above is from hostap machine to athn(4) client. Below is the other way
> > around. Client to hostap. Measurments with mtr on both ends were not
> > executed at the same time, but one after the other, with couple of
> > mintues gap.
> > 
> > # mtr -rwb -c 1000 192.168.220.1
> > Start: 2021-03-31T10:38:07+
> > HOST: pce-0067.home.local Loss%   Snt   Last   Avg  Best  Wrst StDev
> >   1.|-- 192.168.220.1   10.5%  10003.1   2.5   1.5  43.7   2.2
> > 
> > The loss is big, but I didn't notice this too much over short
> > interactive ssh sessions. However I do notice problem havily when I'm on
> > a laptop with iwm(4) ssh'ing to athn(4) client. Then ssh stalls are
> > sigificant that I need to wait until TCP recovers and I can type again.
> > I'm often using scp(1) between athn(4) client and iwm(4) laptop and that
> > amplifies the problem during simultaneous interactive ssh session.
> > SSH stalls are more prominient, longer.
> > 
> > I need to say it's still better than I think without this RA diff, as
> > communitcating with athn(4) client from iwm(4) laptop before was worse
> > as that very often triggered those famous athn device timeouts and
> > recovering from them took way longer than from the packet loss and RA.
> > Packet loss revovery with RA is in tens of seconds, recovery from athn
> > device timeout was in minutes, because usually timeouts occured one
> > after another, like 3 or 4 in a row. With RA I don't see this happening
> > any more.
> > 
> > So, all in all I prefer RA, but I do see packet losses.
> > 
> 
> I did also from athn client to athn hostap:
> 
> # ping -g -c1000 192.168.220.1
> PING 192.168.220.1 (192.168.220.1): 56 data bytes
> .!.!.!.!!.!!.!!!.!.!!
> ..!..
> ..!!!
> .!.!.!.!!.!.!
> !!.!.!....!!!
> ..!.!
> .!.!.!!..!.!

Re: athn(4): switch Tx rate control to RA

[Mikolaj Kucharski]

On Wed, Mar 31, 2021 at 11:24:19AM +, Mikolaj Kucharski wrote:
> On Tue, Mar 23, 2021 at 07:47:08PM +0000, Mikolaj Kucharski wrote:
> > On Tue, Mar 23, 2021 at 07:06:33PM +, Mikolaj Kucharski wrote:
> > > On Tue, Mar 23, 2021 at 06:01:27PM +0100, Stefan Sperling wrote:
> > > > This switches athn(4) to the new RA Tx rate adaptation module.
> > > > Tests on athn(4) PCI devices are welcome.
> > > > USB devices don't need to be tested in this case Tx rate adaptation
> > > > is taken care of by firmware.
> > > > 
> > > > I could only test on AR9285 so far, but the result looks promising.
> > > > 
> > > > diff c6a6a64b49f2287751632205d64f594eb6c1ee42 
> > > > 636e9964c6e5313bb1c75823249d483597a0e93a
> > > ...
> > > 
> > > 
> > > Tested on athn(4) in `mediaopt hostap` mode. Uptime 1hr, so not a lot of
> > > testing yet. So far no issues. I have two systems like that (in hostap)
> > > and in `dmesg | grep athn` only difference is mac address between them.
> > > I can send full dmesg from second system as well if you want.
> > > 
> > 
> > I also have third system, with the same athn(4) card (only mac address
> > is different in `dmesg | grep athn` output), but it acts as a Wi-Fi
> > client and is connected to OpenBSD athn(4)-based access point from
> > my previous email (full dmesg output in an earlier email).
> > 
> 
> After week of running this I have similar observations like Scott
> Bennett.
> 
> I will focus on one location, as I have 2 access points running on
> athn(4) with the diff from this thread, but I'm only present at one of
> those locations. All athn(4) machines have the diff applied.
> 
> OpenBSD, athn(4), hostap
> OpenBSD, athn(4), wi-fi client to above access point
> OpenBSD, iwm(4), wi-fi client to above access point
> 
> I do see some packets dropped with RA patch:
> 
> # mtr -rwb -c 1000 192.168.220.76
> Start: 2021-03-31T08:17:57+
> HOST: pce-0041.home.lan Loss%   Snt   Last   Avg  Best  Wrst StDev
>   1.|-- 192.168.220.76 9.2%  10002.2   2.6   1.2  82.9   3.4
> 
> Above is from hostap machine to athn(4) client. Below is the other way
> around. Client to hostap. Measurments with mtr on both ends were not
> executed at the same time, but one after the other, with couple of
> mintues gap.
> 
> # mtr -rwb -c 1000 192.168.220.1
> Start: 2021-03-31T10:38:07+
> HOST: pce-0067.home.local Loss%   Snt   Last   Avg  Best  Wrst StDev
>   1.|-- 192.168.220.1   10.5%  10003.1   2.5   1.5  43.7   2.2
> 
> The loss is big, but I didn't notice this too much over short
> interactive ssh sessions. However I do notice problem havily when I'm on
> a laptop with iwm(4) ssh'ing to athn(4) client. Then ssh stalls are
> sigificant that I need to wait until TCP recovers and I can type again.
> I'm often using scp(1) between athn(4) client and iwm(4) laptop and that
> amplifies the problem during simultaneous interactive ssh session.
> SSH stalls are more prominient, longer.
> 
> I need to say it's still better than I think without this RA diff, as
> communitcating with athn(4) client from iwm(4) laptop before was worse
> as that very often triggered those famous athn device timeouts and
> recovering from them took way longer than from the packet loss and RA.
> Packet loss revovery with RA is in tens of seconds, recovery from athn
> device timeout was in minutes, because usually timeouts occured one
> after another, like 3 or 4 in a row. With RA I don't see this happening
> any more.
> 
> So, all in all I prefer RA, but I do see packet losses.
> 

I did also from athn client to athn hostap:

# ping -g -c1000 192.168.220.1
PING 192.168.220.1 (192.168.220.1): 56 data bytes
.!.!.!.!!.!!.!!!.!.!!
..!..
..!!!
.!.!.!.!!.!.!
!!.!.!....!!!
..!.!
.!.!.!!..!.!!
!.!.!
!!!.!.!!!.!!!
.!...
!!!.!.!.!.!!!
!.!.!!!.!.!.!!!.!.!!!
!!!.!.!.!..!.!.!!
.
!!.!!!.!.

Re: athn(4): switch Tx rate control to RA

[Mikolaj Kucharski]

On Tue, Mar 23, 2021 at 07:47:08PM +, Mikolaj Kucharski wrote:
> On Tue, Mar 23, 2021 at 07:06:33PM +0000, Mikolaj Kucharski wrote:
> > On Tue, Mar 23, 2021 at 06:01:27PM +0100, Stefan Sperling wrote:
> > > This switches athn(4) to the new RA Tx rate adaptation module.
> > > Tests on athn(4) PCI devices are welcome.
> > > USB devices don't need to be tested in this case Tx rate adaptation
> > > is taken care of by firmware.
> > > 
> > > I could only test on AR9285 so far, but the result looks promising.
> > > 
> > > diff c6a6a64b49f2287751632205d64f594eb6c1ee42 
> > > 636e9964c6e5313bb1c75823249d483597a0e93a
> > ...
> > 
> > 
> > Tested on athn(4) in `mediaopt hostap` mode. Uptime 1hr, so not a lot of
> > testing yet. So far no issues. I have two systems like that (in hostap)
> > and in `dmesg | grep athn` only difference is mac address between them.
> > I can send full dmesg from second system as well if you want.
> > 
> 
> I also have third system, with the same athn(4) card (only mac address
> is different in `dmesg | grep athn` output), but it acts as a Wi-Fi
> client and is connected to OpenBSD athn(4)-based access point from
> my previous email (full dmesg output in an earlier email).
> 

After week of running this I have similar observations like Scott
Bennett.

I will focus on one location, as I have 2 access points running on
athn(4) with the diff from this thread, but I'm only present at one of
those locations. All athn(4) machines have the diff applied.

OpenBSD, athn(4), hostap
OpenBSD, athn(4), wi-fi client to above access point
OpenBSD, iwm(4), wi-fi client to above access point

I do see some packets dropped with RA patch:

# mtr -rwb -c 1000 192.168.220.76
Start: 2021-03-31T08:17:57+
HOST: pce-0041.home.lan Loss%   Snt   Last   Avg  Best  Wrst StDev
  1.|-- 192.168.220.76 9.2%  10002.2   2.6   1.2  82.9   3.4

Above is from hostap machine to athn(4) client. Below is the other way
around. Client to hostap. Measurments with mtr on both ends were not
executed at the same time, but one after the other, with couple of
mintues gap.

# mtr -rwb -c 1000 192.168.220.1
Start: 2021-03-31T10:38:07+
HOST: pce-0067.home.local Loss%   Snt   Last   Avg  Best  Wrst StDev
  1.|-- 192.168.220.1   10.5%  10003.1   2.5   1.5  43.7   2.2

The loss is big, but I didn't notice this too much over short
interactive ssh sessions. However I do notice problem havily when I'm on
a laptop with iwm(4) ssh'ing to athn(4) client. Then ssh stalls are
sigificant that I need to wait until TCP recovers and I can type again.
I'm often using scp(1) between athn(4) client and iwm(4) laptop and that
amplifies the problem during simultaneous interactive ssh session.
SSH stalls are more prominient, longer.

I need to say it's still better than I think without this RA diff, as
communitcating with athn(4) client from iwm(4) laptop before was worse
as that very often triggered those famous athn device timeouts and
recovering from them took way longer than from the packet loss and RA.
Packet loss revovery with RA is in tens of seconds, recovery from athn
device timeout was in minutes, because usually timeouts occured one
after another, like 3 or 4 in a row. With RA I don't see this happening
any more.

So, all in all I prefer RA, but I do see packet losses.

-- 
Regards,
 Mikolaj

Re: athn(4): switch Tx rate control to RA

[Mikolaj Kucharski]

On Tue, Mar 23, 2021 at 08:52:12PM +0100, Stefan Sperling wrote:
> On Tue, Mar 23, 2021 at 07:47:08PM +0000, Mikolaj Kucharski wrote:
> > I also have third system, with the same athn(4) card (only mac address
> > is different in `dmesg | grep athn` output), but it acts as a Wi-Fi
> > client and is connected to OpenBSD athn(4)-based access point from
> > my previous email (full dmesg output in an earlier email).
> 
> In case it wasn't clear, this patch affects both client and hostap modes
> so you'll want to patch both ends.

Yes, sorry I should mention this too. All three systems run kernel with
your patch applied.

-- 
Regards,
 Mikolaj

Re: athn(4): switch Tx rate control to RA

[Mikolaj Kucharski]

On Tue, Mar 23, 2021 at 07:06:33PM +, Mikolaj Kucharski wrote:
> On Tue, Mar 23, 2021 at 06:01:27PM +0100, Stefan Sperling wrote:
> > This switches athn(4) to the new RA Tx rate adaptation module.
> > Tests on athn(4) PCI devices are welcome.
> > USB devices don't need to be tested in this case Tx rate adaptation
> > is taken care of by firmware.
> > 
> > I could only test on AR9285 so far, but the result looks promising.
> > 
> > diff c6a6a64b49f2287751632205d64f594eb6c1ee42 
> > 636e9964c6e5313bb1c75823249d483597a0e93a
> ...
> 
> 
> Tested on athn(4) in `mediaopt hostap` mode. Uptime 1hr, so not a lot of
> testing yet. So far no issues. I have two systems like that (in hostap)
> and in `dmesg | grep athn` only difference is mac address between them.
> I can send full dmesg from second system as well if you want.
> 

I also have third system, with the same athn(4) card (only mac address
is different in `dmesg | grep athn` output), but it acts as a Wi-Fi
client and is connected to OpenBSD athn(4)-based access point from
my previous email (full dmesg output in an earlier email).

-- 
Regards,
 Mikolaj

Re: athn(4): switch Tx rate control to RA

[Mikolaj Kucharski]

On Tue, Mar 23, 2021 at 06:01:27PM +0100, Stefan Sperling wrote:
> This switches athn(4) to the new RA Tx rate adaptation module.
> Tests on athn(4) PCI devices are welcome.
> USB devices don't need to be tested in this case Tx rate adaptation
> is taken care of by firmware.
> 
> I could only test on AR9285 so far, but the result looks promising.
> 
> diff c6a6a64b49f2287751632205d64f594eb6c1ee42 
> 636e9964c6e5313bb1c75823249d483597a0e93a
...


Tested on athn(4) in `mediaopt hostap` mode. Uptime 1hr, so not a lot of
testing yet. So far no issues. I have two systems like that (in hostap)
and in `dmesg | grep athn` only difference is mac address between them.
I can send full dmesg from second system as well if you want.


OpenBSD 6.9-beta (GENERIC.MP) #5: Tue Mar 23 17:36:03 UTC 2021

r...@pc1.home.local:/home/mkucharski/openbsd/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 4259872768 (4062MB)
avail mem = 4115365888 (3924MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xcfe8b020 (13 entries)
bios0: vendor coreboot version "v4.12.0.3" date 07/30/2020
bios0: PC Engines apu3
acpi0 at bios0: ACPI 6.0
acpi0: sleep states S0 S1 S4 S5
acpi0: tables DSDT FACP SSDT MCFG TPM2 APIC HEST SSDT SSDT DRTM HPET
acpi0: wakeup devices PBR4(S4) PBR5(S4) PBR6(S4) PBR7(S4) PBR8(S4) UOH1(S3) 
UOH2(S3) UOH3(S3) UOH4(S3) UOH5(S3) UOH6(S3) XHC0(S4)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xf800, bus 0-64
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD GX-412TC SOC, 998.28 MHz, 16-30-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TOPEXT,DBKP,PERFTSC,PCTRL3,ITSC,BMI1,XSAVEOPT
cpu0: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 2MB 64b/line 
16-way L2 cache
cpu0: ITLB 32 4KB entries fully associative, 8 4MB entries fully associative
cpu0: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD GX-412TC SOC, 998.14 MHz, 16-30-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TOPEXT,DBKP,PERFTSC,PCTRL3,ITSC,BMI1,XSAVEOPT
cpu1: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 2MB 64b/line 
16-way L2 cache
cpu1: ITLB 32 4KB entries fully associative, 8 4MB entries fully associative
cpu1: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: AMD GX-412TC SOC, 998.14 MHz, 16-30-01
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TOPEXT,DBKP,PERFTSC,PCTRL3,ITSC,BMI1,XSAVEOPT
cpu2: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 2MB 64b/line 
16-way L2 cache
cpu2: ITLB 32 4KB entries fully associative, 8 4MB entries fully associative
cpu2: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: AMD GX-412TC SOC, 998.33 MHz, 16-30-01
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TOPEXT,DBKP,PERFTSC,PCTRL3,ITSC,BMI1,XSAVEOPT
cpu3: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 2MB 64b/line 
16-way L2 cache
cpu3: ITLB 32 4KB entries fully associative, 8 4MB entries fully associative
cpu3: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
cpu3: smt 0, core 3, package 0
ioapic0 at mainbus0: apid 4 pa 0xfec0, version 21, 24 pins
ioapic1 at mainbus0: apid 5 pa 0xfec2, version 21, 32 pins
acpihpet0 at acpi0: 14318180 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (PBR4)
acpiprt2 at acpi0: bus 1 (PBR5)
acpiprt3 at acpi0: bus 2 (PBR6)
acpiprt4 at acpi0: bus 3 (PBR7)
acpiprt5 at acpi0: bus 4 (PBR8)
acpipci0 at acpi0 PCI0: 0x 0x0011 0x0001
acpicmos0 at acpi0
amdgpio0 at acpi0 GPIO uid 0 addr 

Re: no IPv6 with umb0 on -current

[Mikolaj Kucharski]

On Sun, Mar 14, 2021 at 12:13:20AM +, Thomas Windisch wrote:
> On Sat, Mar 13, 2021 at 11:34:29AM +, Stuart Henderson wrote:
> > Nothing changed in umb(4) since 6.8-release. The change last year was
> > between 6.7 and 6.8 ("IPv6 is no longer on by default. It must be
> > enabled with "inet6 eui64").
> 
> It seems that the issue I had was not related to OpenBSD but due to 
> bad configuration of the modem. What I failed to mention in my inital
> email was that there were two modems (same model and firmware) on two
> machines: one running -current, the other 6.8-release. I wrongfully
> assumed that it had to do something with the OS. Apparently only one of
> the modems had a setting set for inet4 AND/OR inet6 that is limiting 
> or not overwritten by umb or via MBIM. Both should have had the same 
> factory settings, colour me surprised. After issuing an appropriate 
> AT-command I can now also get IPv6 on my -current machine.
> 

Could you share dmesg, modem model and AT commands you used to configure
it for IPv6?

-- 
Regards,
 Mikolaj

Re: Back-out USB data toggle fix

[Mikolaj Kucharski]

On Sun, Feb 14, 2021 at 03:22:28PM +0100, Marcus Glocker wrote:
> Unfortunately I'm seeing more and more USB device breakages reported
> the last few days related to the USB data toggle fix which we did
> commit 2-3 weeks ago.
> 
> Since I can't reproduce the issue here with my USB gear, it's very
> difficult for me to pin point the issue.  Therefore I think we should
> back it out for now.
> 
> OK?

Could ugen changes stay and just uhidev be reverted?


> Index: lib/libfido2/src/hid_openbsd.c
> ===
> RCS file: /cvs/src/lib/libfido2/src/hid_openbsd.c,v
> retrieving revision 1.6
> diff -u -p -u -p -r1.6 hid_openbsd.c
> --- lib/libfido2/src/hid_openbsd.c5 Feb 2021 14:19:21 -   1.6
> +++ lib/libfido2/src/hid_openbsd.c14 Feb 2021 13:58:22 -
> @@ -88,6 +88,62 @@ fido_hid_manifest(fido_dev_info_t *devli
>   return FIDO_OK;
>  }
>  
> +/*
> + * Workaround for OpenBSD <=6.6-current (as of 201910) bug that loses
> + * sync of DATA0/DATA1 sequence bit across uhid open/close.
> + * Send pings until we get a response - early pings with incorrect
> + * sequence bits will be ignored as duplicate packets by the device.
> + */
> +static int
> +terrible_ping_kludge(struct hid_openbsd *ctx)
> +{
> + u_char data[256];
> + int i, n;
> + struct pollfd pfd;
> +
> + if (sizeof(data) < ctx->report_out_len + 1)
> + return -1;
> + for (i = 0; i < 4; i++) {
> + memset(data, 0, sizeof(data));
> + /* broadcast channel ID */
> + data[1] = 0xff;
> + data[2] = 0xff;
> + data[3] = 0xff;
> + data[4] = 0xff;
> + /* Ping command */
> + data[5] = 0x81;
> + /* One byte ping only, Vasili */
> + data[6] = 0;
> + data[7] = 1;
> + fido_log_debug("%s: send ping %d", __func__, i);
> + if (fido_hid_write(ctx, data, ctx->report_out_len + 1) == -1)
> + return -1;
> + fido_log_debug("%s: wait reply", __func__);
> + memset(, 0, sizeof(pfd));
> + pfd.fd = ctx->fd;
> + pfd.events = POLLIN;
> + if ((n = poll(, 1, 100)) == -1) {
> + fido_log_debug("%s: poll: %s", __func__, 
> strerror(errno));
> + return -1;
> + } else if (n == 0) {
> + fido_log_debug("%s: timed out", __func__);
> + continue;
> + }
> + if (fido_hid_read(ctx, data, ctx->report_out_len, 250) == -1)
> + return -1;
> + /*
> +  * Ping isn't always supported on the broadcast channel,
> +  * so we might get an error, but we don't care - we're
> +  * synched now.
> +  */
> + fido_log_debug("%s: got reply", __func__);
> + fido_log_xxd(data, ctx->report_out_len);
> + return 0;
> + }
> + fido_log_debug("%s: no response", __func__);
> + return -1;
> +}
> +
>  void *
>  fido_hid_open(const char *path)
>  {
> @@ -101,6 +157,16 @@ fido_hid_open(const char *path)
>   ret->report_in_len = ret->report_out_len = MAX_U2FHID_LEN;
>   fido_log_debug("%s: inlen = %zu outlen = %zu", __func__,
>   ret->report_in_len, ret->report_out_len);
> +
> + /*
> +  * OpenBSD (as of 201910) has a bug that causes it to lose
> +  * track of the DATA0/DATA1 sequence toggle across uhid device
> +  * open and close. This is a terrible hack to work around it.
> +  */
> + if (terrible_ping_kludge(ret) != 0) {
> + fido_hid_close(ret);
> + return NULL;
> + }
>  
>   return (ret);
>  }
> Index: sys/dev/usb/ugen.c
> ===
> RCS file: /cvs/src/sys/dev/usb/ugen.c,v
> retrieving revision 1.115
> diff -u -p -u -p -r1.115 ugen.c
> --- sys/dev/usb/ugen.c5 Feb 2021 08:17:22 -   1.115
> +++ sys/dev/usb/ugen.c14 Feb 2021 13:58:26 -
> @@ -117,7 +117,6 @@ int ugen_do_close(struct ugen_softc *, i
>  int ugen_set_config(struct ugen_softc *, int);
>  int ugen_set_interface(struct ugen_softc *, int, int);
>  int ugen_get_alt_index(struct ugen_softc *, int);
> -void ugen_clear_iface_eps(struct ugen_softc *, struct usbd_interface *);
>  
>  #define UGENUNIT(n) ((minor(n) >> 4) & 0xf)
>  #define UGENENDPOINT(n) (minor(n) & 0xf)
> @@ -310,8 +309,6 @@ ugenopen(dev_t dev, int flag, int mode, 
>   DPRINTFN(5, ("ugenopen: sc=%p, endpt=%d, dir=%d, sce=%p\n",
>sc, endpt, dir, sce));
>   edesc = sce->edesc;
> - /* Clear device endpoint toggle. */
> - ugen_clear_iface_eps(sc, sce->iface);
>   switch (UE_GET_XFERTYPE(edesc->bmAttributes)) {
>   case UE_INTERRUPT:
>   if (dir == OUT) {
> @@ -339,8 

sysupgrade ug_err not found

[Mikolaj Kucharski]

Hi,

Diff fixes reference to one of the functions. There is no ug_err(), but
I guess it should be just err(). Discovered via small typo in an URL.

Before:
# sysupgrade -s file:/var/cache/openbsd
/usr/sbin/sysupgrade[112]: ug_err: not found

After:
# sysupgrade -s file:/var/cache/openbsd
sysupgrade: invalid installurl: file:/var/cache/openbsd

Index: sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.43
diff -u -p -u -r1.43 sysupgrade.sh
--- sysupgrade.sh   21 Oct 2020 10:28:49 -  1.43
+++ sysupgrade.sh   22 Oct 2020 07:01:18 -
@@ -109,7 +109,7 @@ case $# in
 *) usage
 esac
 [[ $MIRROR == @(file|ftp|http|https)://* ]] ||
-   ug_err "invalid installurl: $MIRROR"
+   err "invalid installurl: $MIRROR"
 
 if ! $RELEASE && [[ ${#_KERNV[*]} == 2 ]]; then
SNAP=true

-- 
Regards,
 Mikolaj

Re: Improve ure(4) performance

[Mikolaj Kucharski]

On Mon, Aug 03, 2020 at 09:07:39AM -0700, Jonathon Fletcher wrote:
> 
> Assuming no issues with this, I would like to get it into the tree. This
> patch predates Marcus' usbd_abort_pipe patch by a few weeks. Let me know
> if you want an updated patch against HEAD after his patch.
> 
> After reports from multiple people, this patch improves tx performance on
> ure from about 90-110MBps to between about 250MBps (Mikolaj), 290MBps 
> (Joshua), 
> 550MBps (me), 1900MBps (Kevin), depending on ure device, network setup, and
> underlying hardware.
> 

No issues on my end. All works really nice. I managed to have couple of
Zoom calls on ure(4) and card didn't crash like before Jonathon's diff.
I've also updated the tree with the diff after usbd_abort_pipe commit
from mglocker@ and so far I don't see any issues. Network card seems
stable. At the time of the writing machine has 21 hours of uptime.

-- 
Regards,
 Mikolaj

Re: Improve ure(4) performance

[Mikolaj Kucharski]

On Thu, Jul 30, 2020 at 07:58:23AM -0700, Jonathon Fletcher wrote:
> 
> Mikolaj,
> 
> I hope the patch has been stable for you. I do have an update - it appears
> that a splx(s) got lost along the way (from the end of ure_txeof). This
> patch adds that back in and has some minor cleanup (variable name, cleanup
> defines, adjust the setting of flags and buffer sizes based on device type).
> I have been running this for a couple of days now.

Yes, no issues so far. I managed to have on Google Meet call, audio only
and one Zoom call (was surprised it actually worked on OpenBSD, but
needed to fake user agent to Linux). Network card didn't fail like
before your diff during VoIP calls. That's really great. I just compiled
new version of the kernel with your below patch, will reboot my laptop
tonight to switch to that new kernel. Thank you!

> Jonathon
> 
> 
> Index: sys/dev/usb/if_urereg.h
> ===
> RCS file: /cvs/src/sys/dev/usb/if_urereg.h,v
> retrieving revision 1.8
> diff -u -p -u -r1.8 if_urereg.h
> --- sys/dev/usb/if_urereg.h   7 Dec 2019 08:45:28 -   1.8
> +++ sys/dev/usb/if_urereg.h   28 Jul 2020 15:30:41 -
> @@ -494,28 +494,30 @@ struct ure_txpkt {
>  #define URE_ENDPT_TX 1
>  #define URE_ENDPT_MAX2
>  
> -#define  URE_TX_LIST_CNT 8
> +#define  URE_TX_LIST_CNT 1
>  #define  URE_RX_LIST_CNT 1
> -#define  URE_RX_BUF_ALIGNsizeof(uint64_t)
> +#define  URE_TX_BUF_ALIGN4
> +#define  URE_RX_BUF_ALIGN8
>  
> -#define  URE_TXBUFSZ 16384
> -#define  URE_8152_RXBUFSZ16384
> -#define  URE_8153_RXBUFSZ32768
> +#define  URE_TX_BUFSZ16384
> +#define  URE_8152_RX_BUFSZ   16384
> +#define  URE_8153_RX_BUFSZ   32768
>  
>  struct ure_chain {
>   struct ure_softc*uc_sc;
>   struct usbd_xfer*uc_xfer;
>   char*uc_buf;
> - struct mbuf *uc_mbuf;
> - int uc_accum;
> - int uc_idx;
> + uint32_tuc_buflen;
> + uint32_tuc_bufmax;
> + struct mbuf_listuc_mbuf_list;
> + SLIST_ENTRY(ure_chain)  uc_list;
> + uint8_t uc_idx;
>  };
>  
>  struct ure_cdata {
> - struct ure_chaintx_chain[URE_TX_LIST_CNT];
> - struct ure_chainrx_chain[URE_RX_LIST_CNT];
> - int tx_prod;
> - int tx_cnt;
> + struct ure_chainure_rx_chain[URE_RX_LIST_CNT];
> + struct ure_chainure_tx_chain[URE_TX_LIST_CNT];
> + SLIST_HEAD(ure_list_head, ure_chain)ure_tx_free;
>  };
>  
>  struct ure_softc {
> @@ -541,7 +543,7 @@ struct ure_softc {
>  
>   struct timeval  ure_rx_notice;
>   int ure_rxbufsz;
> - int ure_tx_list_cnt;
> + int ure_txbufsz;
>  
>   int ure_phyno;
>  
> Index: sys/dev/usb/if_ure.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_ure.c,v
> retrieving revision 1.16
> diff -u -p -u -r1.16 if_ure.c
> --- sys/dev/usb/if_ure.c  10 Jul 2020 13:26:41 -  1.16
> +++ sys/dev/usb/if_ure.c  28 Jul 2020 22:56:29 -
> @@ -117,11 +117,13 @@ voidure_miibus_writereg(struct device 
>  void ure_lock_mii(struct ure_softc *);
>  void ure_unlock_mii(struct ure_softc *);
>  
> -int  ure_encap(struct ure_softc *, struct mbuf *);
> +int  ure_encap_txpkt(struct mbuf *, char *, uint32_t);
> +int  ure_encap_xfer(struct ifnet *, struct ure_softc *, struct 
> ure_chain *);
>  void ure_rxeof(struct usbd_xfer *, void *, usbd_status);
>  void ure_txeof(struct usbd_xfer *, void *, usbd_status);
> -int  ure_rx_list_init(struct ure_softc *);
> -int  ure_tx_list_init(struct ure_softc *);
> +int  ure_xfer_list_init(struct ure_softc *, struct ure_chain *,
> + uint32_t, int);
> +void ure_xfer_list_free(struct ure_softc *, struct ure_chain *, int);
>  
>  void ure_tick_task(void *);
>  void ure_tick(void *);
> @@ -625,12 +627,36 @@ void
>  ure_watchdog(struct ifnet *ifp)
>  {
>   struct ure_softc*sc = ifp->if_softc;
> + struct ure_chain*c;
> + usbd_status err;
> + int i, s;
> +
> + ifp->if_timer = 0;
>  
>   if (usbd_is_dying(sc->ure_udev))
>   return;
>  
> + if ((ifp->if_flags & (IFF_RUNNING|IFF_UP)) != (IFF_RUNNING|IFF_UP))
> + return;
> +
> + sc = ifp->if_softc;
> + s = splnet();
> +
>   ifp->if_oerrors++;
> - printf("%s: watchdog timeout\n", sc->ure_dev.dv_xname);
> + DPRINTF(("%s: watchdog timeout\n", sc->ure_dev.dv_xname));
> +
> + 

Re: Improve ure(4) performance

[Mikolaj Kucharski]

On Mon, Jul 27, 2020 at 11:58:02AM -0700, Jonathon Fletcher wrote:
> 
> Are you ok using patch's -l option for this patch?
> 

Sure, can do. New kernel compiled, will switch my machine tonight. Will
let you know how things go. For meetings I would need couple of days, as
I don't have Google Meet calls that often.

-- 
Regards,
 Mikolaj

Re: Improve ure(4) performance

[Mikolaj Kucharski]

On Mon, Jul 27, 2020 at 11:24:55AM -0700, Jonathon Fletcher wrote:
> 
> I have attached an updated patch - which includes Kevin's changes from
> his earlier email - that also corrects the tx buffer sizes for RTL8153
> / RTL8153B devices.
> 

Unfortunately below diff fails to apply.

2 out of 2 hunks failed
15 out of 15 hunks failed

I've tried on fresh cvs version of if_urereg.h and if_ure.c

I usually, for convenience take the diffs from marc.info:

https://marc.info/?l=openbsd-tech=159587449018248=mbox

> 
> Index: sys/dev/usb/if_urereg.h
> ===
> RCS file: /cvs/src/sys/dev/usb/if_urereg.h,v
> retrieving revision 1.8
> diff -u -p -u -r1.8 if_urereg.h
> --- sys/dev/usb/if_urereg.h 7 Dec 2019 08:45:28 - 1.8
> +++ sys/dev/usb/if_urereg.h 27 Jul 2020 17:10:09 -
> @@ -494,28 +494,32 @@ struct ure_txpkt {
>  #define URE_ENDPT_TX 1
>  #define URE_ENDPT_MAX 2
> 
> -#define URE_TX_LIST_CNT 8
> +#define URE_TX_LIST_CNT 1
>  #define URE_RX_LIST_CNT 1
> -#define URE_RX_BUF_ALIGN sizeof(uint64_t)
> +#define URE_TX_BUF_ALIGN 4
> +#define URE_RX_BUF_ALIGN 8
> 
> -#define URE_TXBUFSZ 16384
> -#define URE_8152_RXBUFSZ 16384
> -#define URE_8153_RXBUFSZ 32768
> +#define URE_8152_TX_BUFSZ 16384
> +#define URE_8152_RX_BUFSZ 16384
> +
> +#define URE_8153_TX_BUFSZ 16384
> +#define URE_8153_RX_BUFSZ 32768
> 
>  struct ure_chain {
>   struct ure_softc *uc_sc;
>   struct usbd_xfer *uc_xfer;
>   char *uc_buf;
> - struct mbuf *uc_mbuf;
> - int uc_accum;
> - int uc_idx;
> + uint32_t uc_buflen;
> + uint32_t uc_bufmax;
> + struct mbuf_list uc_mbuf_list;
> + SLIST_ENTRY(ure_chain)  ure_list;
> + uint8_t uc_idx;
>  };
> 
>  struct ure_cdata {
> - struct ure_chain tx_chain[URE_TX_LIST_CNT];
> - struct ure_chain rx_chain[URE_RX_LIST_CNT];
> - int tx_prod;
> - int tx_cnt;
> + struct ure_chain ure_rx_chain[URE_RX_LIST_CNT];
> + struct ure_chain ure_tx_chain[URE_TX_LIST_CNT];
> + SLIST_HEAD(ure_list_head, ure_chain)ure_tx_free;
>  };
> 
>  struct ure_softc {
> @@ -541,7 +545,7 @@ struct ure_softc {
> 
>   struct timeval ure_rx_notice;
>   int ure_rxbufsz;
> - int ure_tx_list_cnt;
> + int ure_txbufsz;
> 
>   int ure_phyno;
> 
> Index: sys/dev/usb/if_ure.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_ure.c,v
> retrieving revision 1.16
> diff -u -p -u -r1.16 if_ure.c
> --- sys/dev/usb/if_ure.c 10 Jul 2020 13:26:41 - 1.16
> +++ sys/dev/usb/if_ure.c 27 Jul 2020 17:10:39 -
> @@ -117,11 +117,13 @@ void ure_miibus_writereg(struct device
>  void ure_lock_mii(struct ure_softc *);
>  void ure_unlock_mii(struct ure_softc *);
> 
> -int ure_encap(struct ure_softc *, struct mbuf *);
> +int ure_encap_txpkt(struct mbuf *, char *, uint32_t);
> +int ure_encap_xfer(struct ifnet *, struct ure_softc *, struct ure_chain *);
>  void ure_rxeof(struct usbd_xfer *, void *, usbd_status);
>  void ure_txeof(struct usbd_xfer *, void *, usbd_status);
> -int ure_rx_list_init(struct ure_softc *);
> -int ure_tx_list_init(struct ure_softc *);
> +int ure_xfer_list_init(struct ure_softc *, struct ure_chain *,
> +uint32_t, int);
> +void ure_xfer_list_free(struct ure_softc *, struct ure_chain *, int);
> 
>  void ure_tick_task(void *);
>  void ure_tick(void *);
> @@ -625,12 +627,36 @@ void
>  ure_watchdog(struct ifnet *ifp)
>  {
>   struct ure_softc *sc = ifp->if_softc;
> + struct ure_chain *c;
> + usbd_status err;
> + int i, s;
> +
> + ifp->if_timer = 0;
> 
>   if (usbd_is_dying(sc->ure_udev))
>   return;
> 
> + if ((ifp->if_flags & (IFF_RUNNING|IFF_UP)) != (IFF_RUNNING|IFF_UP))
> + return;
> +
> + sc = ifp->if_softc;
> + s = splnet();
> +
>   ifp->if_oerrors++;
> - printf("%s: watchdog timeout\n", sc->ure_dev.dv_xname);
> + DPRINTF(("watchdog timeout\n"));
> +
> + for (i = 0; i < URE_TX_LIST_CNT; i++) {
> + c = >ure_cdata.ure_tx_chain[i];
> + if (ml_len(>uc_mbuf_list) > 0) {
> + usbd_get_xfer_status(c->uc_xfer, NULL, NULL, NULL,
> +);
> + ure_txeof(c->uc_xfer, c, err);
> + }
> + }
> +
> + if (ifq_is_oactive(>if_snd))
> + ifq_restart(>if_snd);
> + splx(s);
>  }
> 
>  void
> @@ -653,18 +679,26 @@ ure_init(void *xsc)
>   else
>   ure_rtl8153_nic_reset(sc);
> 
> - if (ure_rx_list_init(sc) == ENOBUFS) {
> + if (ure_xfer_list_init(sc, sc->ure_cdata.ure_rx_chain,
> + sc->ure_rxbufsz, URE_RX_LIST_CNT) == ENOBUFS) {
>   printf("%s: rx list init failed\n", sc->ure_dev.dv_xname);
>   splx(s);
>   return;
>   }
> 
> - if (ure_tx_list_init(sc) == ENOBUFS) {
> + if (ure_xfer_list_init(sc, sc->ure_cdata.ure_tx_chain,
> + sc->ure_txbufsz, URE_TX_LIST_CNT) == ENOBUFS) {
>   printf("%s: tx list init failed\n", sc->ure_dev.dv_xname);
>   splx(s);
>   return;
>   }
> 
> + /* Initialize the SLIST we are using for the multiple tx buffers */
> + SLIST_INIT(>ure_cdata.ure_tx_free);
> + for (i = 0; i < URE_TX_LIST_CNT; i++)
> + SLIST_INSERT_HEAD(>ure_cdata.ure_tx_free,
> + >ure_cdata.ure_tx_chain[i], ure_list);
> +
>   /* Set MAC address. */
>   

Re: Improve ure(4) performance

[Mikolaj Kucharski]

On Sun, Jul 26, 2020 at 06:47:32PM -0700, Jonathon Fletcher wrote:
> Mikolaj,
> 
> Thank you for testing my patch.
> 
> I am very interested to know what xhci (or ehci) hardware you have.
> 
> I have the following:
> 
> xhci0 at pci0 dev 20 function 0 "Intel 9 Series xHCI" rev 0x03: msi, xHCI 1.0
> usb0 at xhci0: USB revision 3.0
> uhub0 at usb0 configuration 1 interface 0 "Intel xHCI root hub" rev 3.00/1.00 
> addr 1
> ure0 at uhub0 port 14 configuration 1 interface 0 "Realtek USB 10/100/1G/2.5G 
> LAN" rev 3.20/30.00 addr 3
> ure0: RTL8156 (0x7030), address 00:e0:4c:ab:64:5a
> 
> My ure0 is https://www.amazon.com/gp/product/B07Z8S6PN4 
> 
> 
> I do not get any watchdog errors with this.
> 
> Kevin has been testing my patch and giving me good feedback. He has seen some 
> watchdog errors with an RTL8153B also.
> 
> I am starting to suspect that I have the tx xfer size wrong (too big) for the 
> 8153 / 8153B devices. I am also trying to eliminate the XHCI hardware as a 
> cause.
> 
> Please could you post your dmesg? At a minimum it would help me a lot to see 
> your usb hardware.
> 

Sure, no problem. I'm testing your patch with:

https://www.amazon.co.uk/gp/product/B081YGDBG7/


OpenBSD 6.7-current (GENERIC.MP) #20: Sun Jul 26 19:49:23 UTC 2020

r...@pc1.home.local:/home/mkucharski/openbsd/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8302006272 (7917MB)
avail mem = 8035315712 (7663MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f114000 (64 entries)
bios0: vendor HUAWEI version "2.00" date 11/07/2017
bios0: HUAWEI HUAWEI MateBook X
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP UEFI UEFI ECDT SSDT MSDM SSDT SSDT SSDT ASPT BOOT HPET 
APIC MCFG SSDT WSMT SSDT DBGP DBG2 SSDT SSDT DMAR NHLT FPDT BGRT
acpi0: wakeup devices GLAN(S4) XHC_(S3) XDCI(S4) HDAS(S4) RP01(S4) PXSX(S4) 
RP02(S4) PXSX(S4) RP03(S4) PXSX(S4) RP04(S4) PXSX(S4) RP05(S4) PXSX(S4) 
RP06(S4) PXSX(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpiec0 at acpi0
acpihpet0 at acpi0: 2399 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2595.04 MHz, 06-8e-09
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 24MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.97 MHz, 06-8e-09
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 1 (application processor)
cpu2: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.96 MHz, 06-8e-09
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,MD_CLEAR,TSXFA,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES,MELTDOWN
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 1, core 0, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.97 MHz, 06-8e-09
cpu3: 

Re: Improve ure(4) performance

[Mikolaj Kucharski]

Hi Kevin, Jonathon,

On Tue, Jul 21, 2020 at 02:38:55PM +0800, Kevin Lo wrote:
> Hi,
> 
> Jonathon Fletcher has been helping get the better performance out of ure(4).
> I ran the tcpbench with ure (RTL8156) for 5 minutes:
> 
> 71538432760 bytes sent over 300.999 seconds
> bandwidth min/avg/max/std-dev = 12.071/1901.448/1947.873/135.283 Mbps
> 
> A big thanks to Jonathon for his hard work.

I've tested this on amd64 with following ure(4) card:

ure0 at uhub2 port 2 configuration 1 interface 0 "Realtek USB 10/100/1000 LAN" 
rev 2.10/30.00 addr 4
ure0: RTL8153 (0x5c30), address 00:e0:4c:04:09:7d

I could see speedtest-cli improvement from around 150 Mbit/s before the
diff, to 245 Mbit/s after the diff.

Big thank you for this patch. Will run it for next couple of days.
Before the diff I faced following errors with ure(4):

usbd_start_next: error=5
ure0: watchdog timeout
ure0: usb errors on rx: IOERROR

and unplug/re-plug dance was needed to bring back the network. That
usually happend during Google Meet video call, so I need few days to see
how this diff goes against web video calls.

> ok?
> 
> Index: sys/dev/usb/if_ure.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_ure.c,v
> retrieving revision 1.16
> diff -u -p -u -p -r1.16 if_ure.c
> --- sys/dev/usb/if_ure.c  10 Jul 2020 13:26:41 -  1.16
> +++ sys/dev/usb/if_ure.c  21 Jul 2020 05:37:45 -
> @@ -117,11 +117,13 @@ voidure_miibus_writereg(struct device 
>  void ure_lock_mii(struct ure_softc *);
>  void ure_unlock_mii(struct ure_softc *);
>  
> -int  ure_encap(struct ure_softc *, struct mbuf *);
> +int  ure_encap_txpkt(struct mbuf *, char *, uint32_t);
> +int  ure_encap_xfer(struct ifnet *, struct ure_softc *, struct 
> ure_chain *);
>  void ure_rxeof(struct usbd_xfer *, void *, usbd_status);
>  void ure_txeof(struct usbd_xfer *, void *, usbd_status);
> -int  ure_rx_list_init(struct ure_softc *);
> -int  ure_tx_list_init(struct ure_softc *);
> +int  ure_xfer_list_init(struct ure_softc *, struct ure_chain *,
> + uint32_t, int);
> +void ure_xfer_list_free(struct ure_softc *, struct ure_chain *, int);
>  
>  void ure_tick_task(void *);
>  void ure_tick(void *);
> @@ -625,12 +627,36 @@ void
>  ure_watchdog(struct ifnet *ifp)
>  {
>   struct ure_softc*sc = ifp->if_softc;
> + struct ure_chain*c;
> + usbd_status err;
> + int i, s;
> +
> + ifp->if_timer = 0;
>  
>   if (usbd_is_dying(sc->ure_udev))
>   return;
>  
> + if ((ifp->if_flags & (IFF_RUNNING|IFF_UP)) != (IFF_RUNNING|IFF_UP))
> + return;
> +
> + sc = ifp->if_softc;
> + s = splnet();
> +
>   ifp->if_oerrors++;
> - printf("%s: watchdog timeout\n", sc->ure_dev.dv_xname);
> + DPRINTF(("watchdog timeout\n"));
> +
> + for (i = 0; i < URE_TX_LIST_CNT; i++) {
> + c = >ure_cdata.ure_tx_chain[i];
> + if (ml_len(>uc_mbuf_list) > 0) {
> + usbd_get_xfer_status(c->uc_xfer, NULL, NULL, NULL,
> + );
> + ure_txeof(c->uc_xfer, c, err);
> + }
> + }
> +
> + if (ifq_is_oactive(>if_snd))
> + ifq_restart(>if_snd);
> + splx(s);
>  }
>  
>  void
> @@ -653,18 +679,26 @@ ure_init(void *xsc)
>   else
>   ure_rtl8153_nic_reset(sc);
>  
> - if (ure_rx_list_init(sc) == ENOBUFS) {
> + if (ure_xfer_list_init(sc, sc->ure_cdata.ure_rx_chain,
> + sc->ure_rxbufsz, URE_RX_LIST_CNT) == ENOBUFS) {
>   printf("%s: rx list init failed\n", sc->ure_dev.dv_xname);
>   splx(s);
>   return;
>   }
>  
> - if (ure_tx_list_init(sc) == ENOBUFS) {
> + if (ure_xfer_list_init(sc, sc->ure_cdata.ure_tx_chain,
> + sc->ure_txbufsz, URE_TX_LIST_CNT) == ENOBUFS) {
>   printf("%s: tx list init failed\n", sc->ure_dev.dv_xname);
>   splx(s);
>   return;
>   }
>  
> + /* Initialize the SLIST we are using for the multiple tx buffers */
> + SLIST_INIT(>ure_cdata.ure_tx_free);
> + for (i = 0; i < URE_TX_LIST_CNT; i++)
> + SLIST_INSERT_HEAD(>ure_cdata.ure_tx_free,
> + >ure_cdata.ure_tx_chain[i], ure_list);
> +
>   /* Set MAC address. */
>   ure_write_1(sc, URE_PLA_CRWECR, URE_MCU_TYPE_PLA, URE_CRWECR_CONFIG);
>   ure_write_mem(sc, URE_PLA_IDR, URE_MCU_TYPE_PLA | URE_BYTE_EN_SIX_BYTES,
> @@ -739,9 +773,9 @@ ure_init(void *xsc)
>  
>   /* Start up the receive pipe. */
>   for (i = 0; i < URE_RX_LIST_CNT; i++) {
> - c = >ure_cdata.rx_chain[i];
> + c = >ure_cdata.ure_rx_chain[i];
>   usbd_setup_xfer(c->uc_xfer, sc->ure_ep[URE_ENDPT_RX],
> - c, c->uc_buf, sc->ure_rxbufsz,
> +  

mtx locking against myself panic message with __func__

[Mikolaj Kucharski]

Hi,

I've faced recently couple of panics like:

panic: mtx 0x8211ed38: locking against myself

and modified messages as with the below diff. I guess you may consider
that with the nature of this type of panic, function name don't help
much to norrow down the problem, nonetheless would following diff be
okay to commit?

See https://marc.info/?l=openbsd-bugs=159553956423219=2 for the
reference of the panic.

I've compile and runtime tested this only on amd64.


Index: arch/hppa/hppa/mutex.c
===
RCS file: /cvs/src/sys/arch/hppa/hppa/mutex.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 mutex.c
--- arch/hppa/hppa/mutex.c  23 Apr 2019 13:35:12 -  1.17
+++ arch/hppa/hppa/mutex.c  17 Jul 2020 07:08:54 -
@@ -74,7 +74,7 @@ mtx_enter_try(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
 
asm volatile (
@@ -108,7 +108,7 @@ mtx_enter(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
 
if (mtx->mtx_wantipl != IPL_NONE)
Index: arch/m88k/m88k/mutex.c
===
RCS file: /cvs/src/sys/arch/m88k/m88k/mutex.c,v
retrieving revision 1.1
diff -u -p -u -r1.1 mutex.c
--- arch/m88k/m88k/mutex.c  26 May 2020 11:55:10 -  1.1
+++ arch/m88k/m88k/mutex.c  17 Jul 2020 07:08:55 -
@@ -104,7 +104,7 @@ mtx_enter_try(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
if (mtx->mtx_wantipl != IPL_NONE)
splx(s);
@@ -126,7 +126,7 @@ mtx_enter(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
 
if (mtx->mtx_wantipl != IPL_NONE)
Index: kern/kern_lock.c
===
RCS file: /cvs/src/sys/kern/kern_lock.c,v
retrieving revision 1.71
diff -u -p -u -r1.71 kern_lock.c
--- kern/kern_lock.c5 Mar 2020 09:28:31 -   1.71
+++ kern/kern_lock.c17 Jul 2020 07:08:55 -
@@ -293,7 +293,7 @@ mtx_enter_try(struct mutex *mtx)
owner = atomic_cas_ptr(>mtx_owner, NULL, ci);
 #ifdef DIAGNOSTIC
if (__predict_false(owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
if (owner == NULL) {
membar_enter_after_atomic();
@@ -326,7 +326,7 @@ mtx_enter(struct mutex *mtx)
 
 #ifdef DIAGNOSTIC
if (__predict_false(mtx->mtx_owner == ci))
-   panic("mtx %p: locking against myself", mtx);
+   panic("%s: mtx %p: locking against myself", __func__, mtx);
 #endif
 
if (mtx->mtx_wantipl != IPL_NONE)
Index: kern/kern_rwlock.c
===
RCS file: /cvs/src/sys/kern/kern_rwlock.c,v
retrieving revision 1.45
diff -u -p -u -r1.45 kern_rwlock.c
--- kern/kern_rwlock.c  2 Mar 2020 17:07:49 -   1.45
+++ kern/kern_rwlock.c  17 Jul 2020 07:08:55 -
@@ -170,8 +170,8 @@ rw_enter_diag(struct rwlock *rwl, int fl
case RW_WRITE:
case RW_READ:
if (RW_PROC(curproc) == RW_PROC(rwl->rwl_owner))
-   panic("rw_enter: %s locking against myself",
-   rwl->rwl_name);
+   panic("%s: %s locking against myself",
+   __func__, rwl->rwl_name);
break;
case RW_DOWNGRADE:
/*


-- 
Regards,
 Mikolaj

Re: Use full word for `machine ddbcpu` command in ddb.html

[Mikolaj Kucharski]

Hi,

Would below change be okay?

On Sat, Apr 11, 2020 at 01:18:03PM +, Mikolaj Kucharski wrote:
> Hi,
> 
> I was following this page in the middle of the night and misread `mach`
> as `match` and it took me a while to realize what I am doing wrong. I
> think using full word makes it easier to parse and is less likely to
> make a mistake.
> 
> Index: ddb.html
> ===
> RCS file: /cvs/www/ddb.html,v
> retrieving revision 1.21
> diff -u -p -u -r1.21 ddb.html
> --- ddb.html  27 May 2019 22:55:19 -  1.21
> +++ ddb.html  11 Apr 2020 13:12:19 -
> @@ -42,7 +42,7 @@ When reporting a kernel panic or crash, 
>  If you are at a
>  https://man.openbsd.org/ddb.4;>ddb>
>  prompt, type trace.
> -If you are running SMP, use the mach ddbcpu N command for each
> +If you are running SMP, use the machine ddbcpu N command for 
> each
>  of the N processors you have and repeat the trace
>  command for each processor.
> 

-- 
Regards,
 Mikolaj

Re: sysupgrade: add BUILDINFO file to fetched files

[Mikolaj Kucharski]

On Sat, May 09, 2020 at 10:51:49PM +, Mikolaj Kucharski wrote:
> 
> A bit of explanation with `cat -n install.sub | expand -t 2`
> 
> 
>   1599  
>   1600  # Fetch and verify the set files.
>   1601  for _f in BUILDINFO $_get_sets; do
>   1602$UU && reset_watchdog
>   1603  
> 
> I've put BUILDINFO, but I guess it could be added directly to
> $_get_sets. However I think it needs to be added after baseXX.tgz set,
> so directory /var/db is created before ftp tries to copy it into
> /mnt/var/db.

I confirmed today, this is the case. Directory /mnt/var/db needs to
exist before BUILDINFO is put in place and I'm using extraction of
baseXX.tgz to make sure /mnt/var/db is created in advance.

> 
>   1663  
>   1664# Install the set files.
>   1665for _f in $_get_sets BUILDINFO; do
>   1666  $UU && reset_watchdog
>   1667  _fsrc="$_src/$_f"
> 
> I'm adding it at the end here, to make sure it's before baseXX.tgz, per
> above explanation about /var/db.
> 
>   1672  # Extract the set files and put the kernel files in place.
>   1673  case $_f in
> 
> I just need a basename of the set and I think using $_f doesn't
> introduce any breakage.
> 
> I've tested below diff by fresh install of OpenBSD on amd64. It works
> for me, file /mnt/var/db/installed.BUILDINFO is created during install
> and after reboot file /var/db/installed.BUILDINFO is present on disk.
> 
> Here is output of the part which diff modifies:
> 
> Set name(s)? (or 'abort' or 'done') [done]
> Get/Verify SHA256.sig   100% |**|  2141   00:00
> Signature Verified
> Get/Verify BUILDINFO100% |**|54   00:00
> Get/Verify bsd  100% |**| 18117 KB02:30
> Get/Verify bsd.rd   100% |**| 10109 KB00:29
> Get/Verify base67.tgz   100% |**|   238 MB13:57
> Get/Verify comp67.tgz   100% |**| 74451 KB02:54
> Get/Verify man67.tgz100% |**|  7464 KB00:19
> Get/Verify game67.tgz   100% |**|  2745 KB00:07
> Get/Verify xbase67.tgz  100% |**| 22912 KB00:58
> Get/Verify xshare67.tgz 100% |**|  4499 KB00:10
> Get/Verify xfont67.tgz  100% |**| 39342 KB01:30
> Get/Verify xserv67.tgz  100% |**| 16767 KB00:32
> Installing bsd  100% |**| 18117 KB00:01
> Installing bsd.rd   100% |**| 10109 KB00:00
> Installing base67.tgz   100% |**|   238 MB00:28
> Extracting etc.tgz  100% |**|   261 KB00:00
> Installing comp67.tgz   100% |**| 74451 KB00:17
> Installing man67.tgz100% |**|  7464 KB00:03
> Installing game67.tgz   100% |**|  2745 KB00:00
> Installing xbase67.tgz  100% |**| 22912 KB00:04
> Extracting xetc.tgz 100% |**|  7023   00:00
> Installing xshare67.tgz 100% |**|  4499 KB00:03
> Installing xfont67.tgz  100% |**| 39342 KB00:06
> Installing xserv67.tgz  100% |**| 16767 KB00:02
> Installing BUILDINFO100% |**|54   00:00
> Location of sets? (disk http nfs or 'done') [done]
> 
> 

Below diff fixes upgrade path when BUILDINFO is missing, which can
happen.

Index: distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1150
diff -u -p -u -r1.1150 install.sub
--- distrib/miniroot/install.sub5 Apr 2020 15:15:42 -   1.1150
+++ distrib/miniroot/install.sub10 May 2020 09:27:48 -
@@ -1598,7 +1598,7 @@ install_files() {
_issue="Signature check of SHA256.sig failed" && break
 
# Fetch and verify the set files.
-   for _f in $_get_sets; do
+   for _f in BUILDINFO $_get_sets; do
$UU && reset_watchdog
 
rm -f /tmp/h /tmp/fail
@@ -1662,7 +1662,7 @@ install_files() {
fi
 
# Install the set files.
-   for _f in $_get_sets; do
+   for _f in $_get_sets BUILDINFO; do
$UU && reset_watchdog
_fsrc="$_src/$_f"
 
@@ -1670,13 +1670,20 @@ install_files() {
[[ -f $_tmpsrc/$_f ]] && _fsrc="file://$_tmpsrc/$_f"
 
# Extra

Re: sysupgrade: add BUILDINFO file to fetched files

[Mikolaj Kucharski]

On Sat, May 09, 2020 at 10:51:49PM +, Mikolaj Kucharski wrote:
> @@ -1678,6 +1678,11 @@ install_files() {
>   file:///mnt/var/sysmerge/${_f%%base*}etc.tgz |
>   tar -zxphf - -C /mnt
>   fi
> + ;;
> + BUILDINFO)
> + # Keep BUILDINFO for sysupgrade(8).
> + $_unpriv ftp -D Installing -Vmo - \
> + "$_fsrc" >"/mnt/var/db/installed.BUILDINFO"
>   ;;
>   *)  # Make a backup of the existing ramdisk kernel in the
>   # bsd.rd only download/verify/install case.

This needs more work, above doesn't work with upgrade path, when
BUILDINFO is missing. For example /home/_sysupgrade/ is prepared
via sysupgrade(8) which doesn't yet know how to fetch BUILDINFO.

-- 
Regards,
 Mikolaj

Re: sysupgrade: add BUILDINFO file to fetched files

[Mikolaj Kucharski]

Hi Stuart,

On Thu, Apr 16, 2020 at 11:53:19AM +0100, Stuart Henderson wrote:
> Rather than downloading it and deleting it again, it would be more
> useful if BUILDINFO was kept around after installing. Then sysupgrade
> could check to make sure it isn't going backwards with a future update.
> (e.g. if some malicious mirror or mitm intentionally serves an old
> snapshot [with a good signature] to prevent users getting a security
> fix).
> 
> I started looking at this a while ago and have had this in my tree (I'd
> forgotten about until I just did a cvs up) - maybe worth some more thought 
> (it's not super-robust but I'm not sure if it needs to be..) ENOTIME to
> look at it more now though.

I really like it. I've looked into miniroot directory to implement
bsd.rd part needed for sysupgrade(8) changes. Diff at the end of this
email.

> Index: usr.sbin/sysupgrade/sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.37
> diff -u -p -r1.37 sysupgrade.sh
> --- usr.sbin/sysupgrade/sysupgrade.sh 26 Jan 2020 22:08:36 -  1.37
> +++ usr.sbin/sysupgrade/sysupgrade.sh 16 Apr 2020 10:40:37 -
> @@ -131,6 +131,7 @@ cd ${SETSDIR}
>  
>  echo "Fetching from ${URL}"
>  unpriv -f SHA256.sig ftp -N sysupgrade -Vmo SHA256.sig ${URL}SHA256.sig
> +unpriv -f BUILDINFO ftp -N sysupgrade -Vmo BUILDINFO ${URL}BUILDINFO
>  
>  _KEY=openbsd-${_KERNV[0]%.*}${_KERNV[0]#*.}-base.pub
>  _NEXTKEY=openbsd-${NEXT_VERSION%.*}${NEXT_VERSION#*.}-base.pub
> @@ -147,11 +148,26 @@ esac
>  unpriv -f SHA256 signify -Ve -p "${SIGNIFY_KEY}" -x SHA256.sig -m SHA256
>  rm SHA256.sig
>  
> +unpriv cksum -qC SHA256 BUILDINFO
> +
>  if cmp -s /var/db/installed.SHA256 SHA256 && ! $FORCE; then
>   echo "Already on latest snapshot."
>   exit 0
>  fi
>  
> +if [[ -r /var/db/installed.BUILDINFO ]] && ! $FORCE; then
> + read _skip _skip _oldbuildtime _skip < /var/db/installed.BUILDINFO
> + read _skip _skip _newbuildtime _skip < BUILDINFO
> + if [[ $_newbuildtime -lt $_oldbuildtime ]]; then
> + echo "Snapshot on mirror is older than installed version!"
> + exit 1
> + fi
> + if [[ $_newbuildtime -eq $_oldbuildtime ]]; then
> + echo "Already on latest snapshot? Mismatch between BUILDINFO 
> and SHA256?"
> + exit 1
> + fi
> +fi
> +
>  # INSTALL.*, bsd*, *.tgz
>  SETS=$(sed -n -e 's/^SHA256 (\(.*\)) .*/\1/' \
>  -e '/^INSTALL\./p;/^bsd/p;/\.tgz$/p' SHA256)
> @@ -187,9 +203,14 @@ Set name(s) = done
>  Directory does not contain SHA256.sig. Continue without verification = yes
>  __EOT
>  
> +# XXX should be done in bsd.rd so that this is present for a clean install 
> too
> +cat <<__EOT > /etc/rc.firsttime
> +cp /home/_sysupgrade/BUILDINFO /var/db/installed.BUILDINFO
> +__EOT
> +
>  if ! ${KEEP}; then
>   CLEAN=$(echo SHA256 ${SETS} | sed -e 's/ /,/g')
> - cat <<__EOT > /etc/rc.firsttime
> + cat <<__EOT >> /etc/rc.firsttime
>  rm -f /home/_sysupgrade/{${CLEAN}}
>  __EOT
>  fi
> 

A bit of explanation with `cat -n install.sub | expand -t 2`


  1599  
  1600  # Fetch and verify the set files.
  1601  for _f in BUILDINFO $_get_sets; do
  1602$UU && reset_watchdog
  1603  

I've put BUILDINFO, but I guess it could be added directly to
$_get_sets. However I think it needs to be added after baseXX.tgz set,
so directory /var/db is created before ftp tries to copy it into
/mnt/var/db.

  1663  
  1664# Install the set files.
  1665for _f in $_get_sets BUILDINFO; do
  1666  $UU && reset_watchdog
  1667  _fsrc="$_src/$_f"

I'm adding it at the end here, to make sure it's before baseXX.tgz, per
above explanation about /var/db.

  1672  # Extract the set files and put the kernel files in place.
  1673  case $_f in

I just need a basename of the set and I think using $_f doesn't
introduce any breakage.

I've tested below diff by fresh install of OpenBSD on amd64. It works
for me, file /mnt/var/db/installed.BUILDINFO is created during install
and after reboot file /var/db/installed.BUILDINFO is present on disk.

Here is output of the part which diff modifies:

Set name(s)? (or 'abort' or 'done') [done]
Get/Verify SHA256.sig   100% |**|  2141   00:00
Signature Verified
Get/Verify BUILDINFO100% |**|54   00:00
Get/Verify bsd  100% |**| 18117 KB02:30
Get/Verify bsd.rd   100% |**| 10109 KB00:29
Get/Verify base67.tgz   100% |**|   238 MB13:57
Get/Verify comp67.tgz   100% |**| 74451 KB02:54
Get/Verify man67.tgz100% |**|  7464 KB00:19
Get/Verify game67.tgz   100% |**|  2745 KB00:07
Get/Verify xbase67.tgz  100% |**| 22912 KB00:58
Get/Verify xshare67.tgz 100% 

Re: sysupgrade: add BUILDINFO file to fetched files

[Mikolaj Kucharski]

Hi,

Would below be okay?

On Tue, Apr 07, 2020 at 04:46:38AM +, Mikolaj Kucharski wrote:
> Hi,
> 
> When I'm upgrading my machines, I find it useful to have BUILDINFO
> file around. Tested on RPi3.
> 
> Please carbon-copy me in any replies. Thank you.
> 
> openbsd-rpi# sysupgrade -s -n
> Fetching from https://cdn.openbsd.org/pub/OpenBSD/snapshots/arm64/
> SHA256.sig   100% |**|  1453   00:00
> Signature Verified
> Verifying old sets.
> BUILDINFO100% |**|54   00:00
> Verifying sets.
> Fetching updated firmware.
> Will upgrade on next reboot
> 
> openbsd-rpi# reboot
> ... [successfull upgrade]
> openbsd-rpi# ls -1A /home/_sysupgrade/ | wc -l
>0
> 
> 
> OpenBSD 6.6-current (GENERIC.MP) #513: Wed Mar 18 16:41:35 MDT 2020
> dera...@arm64.openbsd.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP
> 
> 
> Index: sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.37
> diff -u -p -u -r1.37 sysupgrade.sh
> --- sysupgrade.sh 26 Jan 2020 22:08:36 -  1.37
> +++ sysupgrade.sh 20 Mar 2020 06:30:51 -
> @@ -152,9 +152,9 @@ if cmp -s /var/db/installed.SHA256 SHA25
>   exit 0
>  fi
>  
> -# INSTALL.*, bsd*, *.tgz
> +# BUILDINFO, INSTALL.*, bsd*, *.tgz
>  SETS=$(sed -n -e 's/^SHA256 (\(.*\)) .*/\1/' \
> --e '/^INSTALL\./p;/^bsd/p;/\.tgz$/p' SHA256)
> +-e '/^BUILDINFO$/p;/^INSTALL\./p;/^bsd/p;/\.tgz$/p' SHA256)
>  
>  OLD_FILES=$(ls)
>  OLD_FILES=$(rmel SHA256 $OLD_FILES)
> 

-- 
Regards,
 Mikolaj

Use full word for `machine ddbcpu` command in ddb.html

[Mikolaj Kucharski]

Hi,

I was following this page in the middle of the night and misread `mach`
as `match` and it took me a while to realize what I am doing wrong. I
think using full word makes it easier to parse and is less likely to
make a mistake.

Index: ddb.html
===
RCS file: /cvs/www/ddb.html,v
retrieving revision 1.21
diff -u -p -u -r1.21 ddb.html
--- ddb.html27 May 2019 22:55:19 -  1.21
+++ ddb.html11 Apr 2020 13:12:19 -
@@ -42,7 +42,7 @@ When reporting a kernel panic or crash, 
 If you are at a
 https://man.openbsd.org/ddb.4;>ddb>
 prompt, type trace.
-If you are running SMP, use the mach ddbcpu N command for each
+If you are running SMP, use the machine ddbcpu N command for 
each
 of the N processors you have and repeat the trace
 command for each processor.

-- 
Regards,
 Mikolaj

sysupgrade: add BUILDINFO file to fetched files

[Mikolaj Kucharski]

Hi,

When I'm upgrading my machines, I find it useful to have BUILDINFO
file around. Tested on RPi3.

Please carbon-copy me in any replies. Thank you.

openbsd-rpi# sysupgrade -s -n
Fetching from https://cdn.openbsd.org/pub/OpenBSD/snapshots/arm64/
SHA256.sig   100% |**|  1453   00:00
Signature Verified
Verifying old sets.
BUILDINFO100% |**|54   00:00
Verifying sets.
Fetching updated firmware.
Will upgrade on next reboot

openbsd-rpi# reboot
... [successfull upgrade]
openbsd-rpi# ls -1A /home/_sysupgrade/ | wc -l
   0


OpenBSD 6.6-current (GENERIC.MP) #513: Wed Mar 18 16:41:35 MDT 2020
dera...@arm64.openbsd.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP


Index: sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.37
diff -u -p -u -r1.37 sysupgrade.sh
--- sysupgrade.sh   26 Jan 2020 22:08:36 -  1.37
+++ sysupgrade.sh   20 Mar 2020 06:30:51 -
@@ -152,9 +152,9 @@ if cmp -s /var/db/installed.SHA256 SHA25
exit 0
 fi
 
-# INSTALL.*, bsd*, *.tgz
+# BUILDINFO, INSTALL.*, bsd*, *.tgz
 SETS=$(sed -n -e 's/^SHA256 (\(.*\)) .*/\1/' \
--e '/^INSTALL\./p;/^bsd/p;/\.tgz$/p' SHA256)
+-e '/^BUILDINFO$/p;/^INSTALL\./p;/^bsd/p;/\.tgz$/p' SHA256)
 
 OLD_FILES=$(ls)
 OLD_FILES=$(rmel SHA256 $OLD_FILES)

-- 
Regards,
 Mikolaj

Re: once again: iwm(4) multi-frame rx + monitor mode

[Mikolaj Kucharski]

Hi Stefan,

On Fri, Sep 13, 2019 at 02:10:12PM +0200, Stefan Sperling wrote:
> Updated diff, rebased on top of -current
> 
> If you were seeing throughput problems when testing this diff in the
> previous round, please try again.
> 
> I suspect such problems were due to the ifq drop issue which has since
> been fixed in -current. The diff below results in more packets per interrupt
> being delivered to the network stack which is exactly what made ifq drops
> more likely to occur. This should no longer be an issue.
> 
> I have tested this diff on 8265 and 7260 and I don't see any problems.

iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x78, msi
iwm0: hw rev 0x230, fw ver 22.361476.0, address 38:37:8b:XX:XX:XX

I've tested monitor mode and it seems to work:

$ ifconfig iwm0
iwm0: flags=8847 mtu 1500
lladdr 38:37:8b:XX:XX:XX
index 1 priority 4 llprio 3
groups: wlan egress
media: IEEE802.11 autoselect monitor
status: active
ieee80211: nwid linksys chan 11 bssid 00:1d:7e:XX:XX:XX 29%

I've used kismet-201607R1p0 package and I could see networks and packets
being reported. Testing with tcpdump(8) also showed traffic visible from
various access points around:

# tcpdump -c 3 -y IEEE802_11_RADIO -ni iwm0  
tcpdump: listening on iwm0, link-type IEEE802_11_RADIO
21:25:12.586606 802.11: beacon, ssid (net_092382), rates, ds, tim, xrates, rsn, 
htcaps, 
21:25:12.653411 802.11: beacon, ssid (linksys), rates, ds, tim, erp, 47:1, 
xrates, vendor, 
21:25:12.755803 802.11: beacon, ssid (linksys), rates, ds, tim, erp, 47:1, 
xrates, vendor, 

I see in tcpdump output that all beacons are reported with chan 11,
where I know some of the access points are not on channel 11. Not
sure is this expected. Other than that, I don't see anything
concerning.

Regards,
 Mikolaj

> diff refs/heads/master refs/heads/iwm-multirx
> blob - 20ce2cbe3d2c2994b9a498965a3c9f29a5f31c9c
> blob + d3baf8473bd86beb05eaed7eb6cb715f90790c46
> --- sys/dev/pci/if_iwm.c
> +++ sys/dev/pci/if_iwm.c
> @@ -367,8 +367,10 @@ int  iwm_get_signal_strength(struct iwm_softc *, 
> struct
>  void iwm_rx_rx_phy_cmd(struct iwm_softc *, struct iwm_rx_packet *,
>   struct iwm_rx_data *);
>  int  iwm_get_noise(const struct iwm_statistics_rx_non_phy *);
> -void iwm_rx_rx_mpdu(struct iwm_softc *, struct iwm_rx_packet *,
> - struct iwm_rx_data *, struct mbuf_list *);
> +int  iwm_rx_frame(struct iwm_softc *, struct mbuf *, uint32_t,
> + struct mbuf_list *);
> +int  iwm_ccmp_decap(struct iwm_softc *, struct mbuf *,
> + struct ieee80211_node *);
>  void iwm_enable_ht_cck_fallback(struct iwm_softc *, struct iwm_node *);
>  void iwm_rx_tx_cmd_single(struct iwm_softc *, struct iwm_rx_packet *,
>   struct iwm_node *);
> @@ -429,7 +431,7 @@ uint8_t   iwm_ridx2rate(struct ieee80211_rateset *, int)
>  int  iwm_rval2ridx(int);
>  void iwm_ack_rates(struct iwm_softc *, struct iwm_node *, int *, int *);
>  void iwm_mac_ctxt_cmd_common(struct iwm_softc *, struct iwm_node *,
> - struct iwm_mac_ctx_cmd *, uint32_t, int);
> + struct iwm_mac_ctx_cmd *, uint32_t);
>  void iwm_mac_ctxt_cmd_fill_sta(struct iwm_softc *, struct iwm_node *,
>   struct iwm_mac_data_sta *, int);
>  int  iwm_mac_ctxt_cmd(struct iwm_softc *, struct iwm_node *, uint32_t, int);
> @@ -470,6 +472,11 @@ const char *iwm_desc_lookup(uint32_t);
>  void iwm_nic_error(struct iwm_softc *);
>  void iwm_nic_umac_error(struct iwm_softc *);
>  #endif
> +void iwm_rx_mpdu(struct iwm_softc *, struct mbuf *, size_t,
> + struct mbuf_list *);
> +int  iwm_rx_pkt_valid(struct iwm_rx_packet *);
> +void iwm_rx_pkt(struct iwm_softc *, struct iwm_rx_data *,
> + struct mbuf_list *);
>  void iwm_notif_intr(struct iwm_softc *);
>  int  iwm_intr(void *);
>  int  iwm_match(struct device *, void *, void *);
> @@ -1723,7 +1730,6 @@ iwm_nic_rx_init(struct iwm_softc *sc)
>   IWM_FH_RCSR_RX_CONFIG_CHNL_EN_ENABLE_VAL|
>   IWM_FH_RCSR_CHNL0_RX_IGNORE_RXF_EMPTY   |  /* HW bug */
>   IWM_FH_RCSR_CHNL0_RX_CONFIG_IRQ_DEST_INT_HOST_VAL   |
> - IWM_FH_RCSR_CHNL0_RX_CONFIG_SINGLE_FRAME_MSK|
>   (IWM_RX_RB_TIMEOUT << IWM_FH_RCSR_RX_CONFIG_REG_IRQ_RBTH_POS) |
>   IWM_FH_RCSR_RX_CONFIG_REG_VAL_RB_SIZE_4K|
>   IWM_RX_QUEUE_SIZE_LOG << IWM_FH_RCSR_RX_CONFIG_RBDCB_SIZE_POS);
> @@ -3429,56 +3435,24 @@ iwm_get_noise(const struct iwm_statistics_rx_non_phy *
>   return (nbant == 0) ? -127 : (total / nbant) - 107;
>  }
>  
> -void
> -iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_packet *pkt,
> -struct iwm_rx_data *data, struct mbuf_list *ml)
> +int
> +iwm_rx_frame(struct iwm_softc *sc, struct mbuf *m, uint32_t rx_pkt_status,
> +struct mbuf_list *ml)
>  {
>   struct ieee80211com *ic = >sc_ic;
>   struct ieee80211_frame *wh;
>   struct ieee80211_node *ni;
>   struct ieee80211_rxinfo rxi;
>   

Re: Should whois(1) and IPv6 default to ANICHOST?

[Mikolaj Kucharski]

Hi,

Ping?

Initial email at https://marc.info/?l=openbsd-tech=152684229010897=2

Regards,
 Mikolaj

On Thu, Jun 07, 2018 at 07:47:17AM +0100, Mikolaj Kucharski wrote:
> Hi,
> 
> Any comments?
> 
> On Sun, May 20, 2018 at 07:48:34PM +0100, Mikolaj Kucharski wrote:
> > Hi,
> > 
> > This is very naive patch for whois(1) which makes it work
> > by default for IPv6 addresses. I went with very minimal
> > approach here. If string contains colon, it's asumed to
> > be IPv6 address. Comments welcome.
> > 
> > Please CC me, as I'm not subscribed to the list.
> > 
> > Index: whois.c
> > ===
> > RCS file: /cvs/src/usr.bin/whois/whois.c,v
> > retrieving revision 1.56
> > diff -u -p -u -r1.56 whois.c
> > --- whois.c 26 Jul 2017 15:48:38 -  1.56
> > +++ whois.c 4 Apr 2018 19:01:45 -
> > @@ -278,7 +278,8 @@ whois(const char *query, const char *ser
> >   * If the TLD is a number, query ARIN, otherwise, use TLD.whois-server.net.
> >   * If the domain does not contain '.', check to see if it is an NSI handle
> >   * (starts with '!') or a CORE handle (COCO-[0-9]+ or COHO-[0-9]+) or an
> > - * ASN (starts with AS). Fall back to NICHOST for the non-handle case.
> > + * ASN (starts with AS) or IPv6 address (contains ':'). Fall back to
> > + * NICHOST for the non-handle and non-IPv6 case.
> >   */
> >  char *
> >  choose_server(const char *name, const char *country, char **tofree)
> > @@ -305,6 +306,8 @@ choose_server(const char *name, const ch
> > else if ((strncasecmp(name, "AS", 2) == 0) &&
> > strtol(name + 2, , 10) > 0 && *ep == '\0')
> > return (MNICHOST);
> > +   else if (strchr(name, ':') != NULL) /* IPv6 address */
> > +   return (ANICHOST);
> > else
> > return (NICHOST);
> > } else if (isdigit((unsigned char)*(++qhead)))

Re: Should whois(1) and IPv6 default to ANICHOST?

[Mikolaj Kucharski]

Hi,

Any comments?

On Sun, May 20, 2018 at 07:48:34PM +0100, Mikolaj Kucharski wrote:
> Hi,
> 
> This is very naive patch for whois(1) which makes it work
> by default for IPv6 addresses. I went with very minimal
> approach here. If string contains colon, it's asumed to
> be IPv6 address. Comments welcome.
> 
> Please CC me, as I'm not subscribed to the list.
> 
> Index: whois.c
> ===
> RCS file: /cvs/src/usr.bin/whois/whois.c,v
> retrieving revision 1.56
> diff -u -p -u -r1.56 whois.c
> --- whois.c   26 Jul 2017 15:48:38 -  1.56
> +++ whois.c   4 Apr 2018 19:01:45 -
> @@ -278,7 +278,8 @@ whois(const char *query, const char *ser
>   * If the TLD is a number, query ARIN, otherwise, use TLD.whois-server.net.
>   * If the domain does not contain '.', check to see if it is an NSI handle
>   * (starts with '!') or a CORE handle (COCO-[0-9]+ or COHO-[0-9]+) or an
> - * ASN (starts with AS). Fall back to NICHOST for the non-handle case.
> + * ASN (starts with AS) or IPv6 address (contains ':'). Fall back to
> + * NICHOST for the non-handle and non-IPv6 case.
>   */
>  char *
>  choose_server(const char *name, const char *country, char **tofree)
> @@ -305,6 +306,8 @@ choose_server(const char *name, const ch
>   else if ((strncasecmp(name, "AS", 2) == 0) &&
>   strtol(name + 2, , 10) > 0 && *ep == '\0')
>   return (MNICHOST);
> + else if (strchr(name, ':') != NULL) /* IPv6 address */
> + return (ANICHOST);
>   else
>   return (NICHOST);
>   } else if (isdigit((unsigned char)*(++qhead)))

Regards,
 Mikolaj

Should whois(1) and IPv6 default to ANICHOST?

[Mikolaj Kucharski]

Hi,

This is very naive patch for whois(1) which makes it work
by default for IPv6 addresses. I went with very minimal
approach here. If string contains colon, it's asumed to
be IPv6 address. Comments welcome.

Please CC me, as I'm not subscribed to the list.

Index: whois.c
===
RCS file: /cvs/src/usr.bin/whois/whois.c,v
retrieving revision 1.56
diff -u -p -u -r1.56 whois.c
--- whois.c 26 Jul 2017 15:48:38 -  1.56
+++ whois.c 4 Apr 2018 19:01:45 -
@@ -278,7 +278,8 @@ whois(const char *query, const char *ser
  * If the TLD is a number, query ARIN, otherwise, use TLD.whois-server.net.
  * If the domain does not contain '.', check to see if it is an NSI handle
  * (starts with '!') or a CORE handle (COCO-[0-9]+ or COHO-[0-9]+) or an
- * ASN (starts with AS). Fall back to NICHOST for the non-handle case.
+ * ASN (starts with AS) or IPv6 address (contains ':'). Fall back to
+ * NICHOST for the non-handle and non-IPv6 case.
  */
 char *
 choose_server(const char *name, const char *country, char **tofree)
@@ -305,6 +306,8 @@ choose_server(const char *name, const ch
else if ((strncasecmp(name, "AS", 2) == 0) &&
strtol(name + 2, , 10) > 0 && *ep == '\0')
return (MNICHOST);
+   else if (strchr(name, ':') != NULL) /* IPv6 address */
+   return (ANICHOST);
else
return (NICHOST);
} else if (isdigit((unsigned char)*(++qhead)))

Re: openvpn-2.3.8p1 segv in libcrypto BN_bn2dec on OpenBSD/i386 current Sep 16, 2015

[Mikolaj Kucharski]

Hi,

On Thu, Sep 17, 2015 at 11:19:59PM +0100, Stuart Henderson wrote:
> CC'ing tech@.
> 
> The last commit to bn_print.c is wrong, it dereferences t while it's still 
> NULL.
> 
> Backout diff below.

Thanks Stuart, recompiling libcrypto with your diff fixed the problem.
OpenVPN doesn't crash any more. Thanks.

-- 
best regards
q#

Re: kernel page fault trap on OpenBSD 5.3 i386 GENERIC #45

[Mikolaj Kucharski]

You may be interested in the my network setup. You can see parts which
are live and parts which are leftovers from my tests. If interface is
avtive, it's probably needed, if it's no carrier status, I was
testing something in the past (in case you are wondering).


# ifconfig
lo0: flags=8049UP,LOOPBACK,RUNNING,MULTICAST mtu 33196
priority: 0
groups: lo
inet6 ::1 prefixlen 128
inet6 fe80::1%lo0 prefixlen 64 scopeid 0xa
inet 127.0.0.1 netmask 0xff00
vr0: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST mtu 1500
lladdr 00:00:24:cc:e5:7c
description: physical link to modem
priority: 0
media: Ethernet autoselect (100baseTX full-duplex)
status: active
inet6 fe80::200:24ff:fecc:e57c%vr0 prefixlen 64 scopeid 0x1
inet 192.168.1.250 netmask 0xff00 broadcast 192.168.1.255
vr1: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST mtu 1500
lladdr 00:00:24:cc:e5:7d
description: link to net4511
priority: 0
media: Ethernet autoselect (none)
status: no carrier
inet 192.168.101.1 netmask 0xff00 broadcast 192.168.101.255
inet6 fe80::200:24ff:fecc:e57d%vr1 prefixlen 64 scopeid 0x2
vr2: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST mtu 1500
lladdr 00:00:24:cc:e5:7e
description: link to linksys
priority: 0
media: Ethernet autoselect (none)
status: no carrier
inet 192.168.102.1 netmask 0xff00 broadcast 192.168.102.255
inet6 fe80::200:24ff:fecc:e57e%vr2 prefixlen 64 scopeid 0x3
vr3: flags=8843UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST mtu 1500
lladdr 00:00:24:cc:e5:7f
description: link to netgear switch
priority: 0
media: Ethernet autoselect (none)
status: no carrier
inet 192.168.103.1 netmask 0xff00 broadcast 192.168.103.255
inet6 fe80::200:24ff:fecc:e57f%vr3 prefixlen 64 scopeid 0x4
sis0: flags=8b43UP,BROADCAST,RUNNING,PROMISC,ALLMULTI,SIMPLEX,MULTICAST mtu 
1500
lladdr 00:00:24:cc:d4:7c
description: part of bridge0
priority: 0
media: Ethernet autoselect (none)
status: no carrier
inet6 fe80::200:24ff:fecc:d47c%sis0 prefixlen 64 scopeid 0x5
sis1: flags=8b43UP,BROADCAST,RUNNING,PROMISC,ALLMULTI,SIMPLEX,MULTICAST mtu 
1500
lladdr 00:00:24:cc:d4:7d
description: part of bridge0
priority: 0
media: Ethernet autoselect (none)
status: no carrier
inet6 fe80::200:24ff:fecc:d47d%sis1 prefixlen 64 scopeid 0x6
sis2: flags=8b43UP,BROADCAST,RUNNING,PROMISC,ALLMULTI,SIMPLEX,MULTICAST mtu 
1500
lladdr 00:00:24:cc:d4:7e
description: part of bridge0
priority: 0
media: Ethernet autoselect (none)
status: no carrier
inet6 fe80::200:24ff:fecc:d47e%sis2 prefixlen 64 scopeid 0x7
sis3: flags=8b43UP,BROADCAST,RUNNING,PROMISC,ALLMULTI,SIMPLEX,MULTICAST mtu 
1500
lladdr 00:00:24:cc:d4:7f
description: part of bridge0
priority: 0
media: Ethernet autoselect (100baseTX full-duplex)
status: active
inet6 fe80::200:24ff:fecc:d47f%sis3 prefixlen 64 scopeid 0x8
enc0: flags=0
priority: 0
groups: enc
status: active
vether0: flags=8943UP,BROADCAST,RUNNING,PROMISC,SIMPLEX,MULTICAST mtu 1500
lladdr fe:e1:ba:d0:55:5f
description: part of bridge0
priority: 0
groups: vether
media: Ethernet autoselect
status: active
inet 192.168.110.1 netmask 0xff00 broadcast 192.168.110.255
inet6 fe80::fce1:baff:fed0:555f%vether0 prefixlen 64 scopeid 0xb
pppoe0: flags=8851UP,POINTOPOINT,RUNNING,SIMPLEX,MULTICAST mtu 1492
description: pppoe uplink
priority: 0
dev: vr0 state: session
sid: 0x3c7a PADI retries: 0 PADR retries: 0 time: 08:02:34
sppp: phase network authproto chap authname eir...@eircom.net
groups: pppoe egress
status: active
inet6 fe80::200:24ff:fecc:e57c%pppoe0 -  prefixlen 64 scopeid 0xc
inet 86.41.44.127 -- 159.134.155.5 netmask 0x
tun0: flags=51UP,POINTOPOINT,RUNNING mtu 1500
priority: 0
groups: tun
status: down
inet 192.168.201.1 -- 192.168.201.2 netmask 0xfffc
tun1: flags=10POINTOPOINT mtu 1500
priority: 0
groups: tun
status: down
gif0: flags=8010POINTOPOINT,MULTICAST mtu 1280
priority: 0
groups: gif
gif1: flags=8051UP,POINTOPOINT,RUNNING,MULTICAST mtu 1280
priority: 0
groups: gif
physical address inet 192.168.101.1 -- 192.168.1.105
inet6 fe80::200:24ff:fecc:e57c%gif1 -  prefixlen 64 scopeid 0x10
bridge0: flags=4041UP,RUNNING,LINK2
description: link to virtual hosts
groups: bridge
priority 32768 hellotime 2 fwddelay 15 maxage 20 holdcnt 6 proto rstp
vether0 flags=3LEARNING,DISCOVER
port 11 

kernel page fault trap on OpenBSD 5.3 i386 GENERIC #45

[Mikolaj Kucharski]

Hi,

Upgraded my Soekris net5501 to the latest snapshot:

OpenBSD 5.3 (GENERIC) #45: Wed Feb 27 13:42:28 MST 2013
dera...@i386.openbsd.org:/usr/src/sys/arch/i386/compile/GENERIC


and during one of the reboots got this:


Automatic boot in progress: starting file system checks.
/dev/rwd0a: file system is clean; not checking
/dev/rwd0g: file system is clean; not checking
/dev/rwd0f: file system is clean; not checking
/dev/rwd0d: file system is clean; not checking
/dev/rwd0e: file system is clean; not checking
setting tty flags
pf enabled
ddb.panic: 1 - 1
ddb.console: 0 - 1
net.inet.ip.forwarding: 0 - 1
starting network
starting early daemons: syslogd nameduvm_fault(0xd0a2dd40, 0xd5d7c000, 0, 1) -
e
kernel: page fault trap, code=0
Stopped at  bcopy+0x1a: repe movsl  (%esi),%es:(%edi)
ddb ps
   PID   PPID   PGRPUID  S   FLAGS  WAIT  COMMAND
  1637   3787   3787 70  3   0  biowait   named
  3787   5609   3787  0  30x80  netio named
  5609  23313  29579  0  30x80  piperdnamed
 23313  24670  29579  0  30x88  pause sh
 24670  29579  29579  0  30x88  pause sh
   279  26321  26321 73  3   0  biowait   syslogd
 26321  1  26321  0  30x80  netio syslogd
 29579  1  29579  0  30x88  pause sh
13  0  0  0  30x100200  aiodoned  aiodoned
12  0  0  0  30x100200  syncerupdate
11  0  0  0  30x100200  cleaner   cleaner
10  0  0  0  30x100200  reaperreaper
 9  0  0  0  30x100200  pgdaemon  pagedaemon
 8  0  0  0  30x100200  bored crypto
 7  0  0  0  30x100200  pftm  pfpurge
 6  0  0  0  30x100200  usbtskusbtask
 5  0  0  0  30x100200  usbatsk   usbatsk
 4  0  0  0  30x100200  bored syswq
*3  0  0  0  7  0x40100200idle0
 2  0  0  0  30x100200  kmalloc   kmthread
 1  0  1  0  30x80  wait  init
 0 -1  0  0  3   0x200  scheduler swapper
ddb trace
bcopy(d5cd1100,96000,3b9aca00,2,1) at bcopy+0x1a
m_copym2(d5cd1100,0,3b9aca00,2,46) at m_copym2+0x2e
bridge_input(d15ab434,d5ce5800,d5cd1100,f5395e70,d02030dd) at bridge_input+0x2e
1
ether_input(d15ab434,0,d5cd1100,800,2) at ether_input+0x39d
sis_rxeof(d15ab400,d300,10,f5395f0c,d15eda00) at sis_rxeof+0x1a6
sis_intr(d15ab400) at sis_intr+0x148
Xrecurse_legacy6() at Xrecurse_legacy6+0xb7
--- interrupt ---
cpu_idle_cycle(d0b0e7c0) at cpu_idle_cycle+0xf
Bad frame pointer: 0xd0bc9e28
ddb



Then tried to reboot the box:


ddb boot reboot
panic: wdc_exec_command: polled command not done
Stopped at  Debugger+0x4:   popl%ebp
RUN AT LEAST 'trace' AND 'ps' AND INCLUDE OUTPUT WHEN REPORTING THIS
PANIC!
DO NOT EVEN BOTHER REPORTING THIS WITHOUT INCLUDING THAT INFORMATION!
ddb trace
Debugger(d08fdc7c,f5395a4c,d089b5d8,f5395a4c,d5d64060) at Debugger+0x4
panic(d089b5d8,d5d64060,f5395a70,d05b8034,0) at panic+0x5d
wdc_exec_command(d1671110,f5395a84,f5395aa0,e7,4000) at
wdc_exec_command+0x
10d
wd_flushcache(d1673c00,10,0,d162f900,d15ab000) at wd_flushcache+0x7e
wdactivate(d1673c00,5,0,d0412b43,0) at wdactivate+0xb3
config_activate_children(d1671000,5,6206,0,c000) at
config_activate_children+0x
45
config_activate_children(d1472e00,5,0,d155ef80,d1472e00) at
config_activate_chi
ldren+0x45
pciactivate(d1472e00,5,f5395b80,d03eb38d,0) at pciactivate+0x70
config_activate_children(d155efc0,5,f5395bb0,d03b7155,d) at
config_activate_chi
ldren+0x45
boot(4804,f5395be4,f5395c70,d03b5014,d02005ee) at boot+0x5a
db_boot_reboot_cmd(d02005ee,0,,f5395be8,0) at
db_boot_reboot_cmd+0x12
db_command(d09e4c00,d09e4a20,d144f663,d02005ee,f5395d6c) at
db_command+0x124
db_command_loop(d02005ee,f5395cd8,f5395ce0,d03bf23d,800) at
db_command_loop+0x7
1
db_trap(6,0,58,6,d5d7c000) at db_trap+0xc0
kdb_trap(6,0,f5395d6c,1,e) at kdb_trap+0xc7
trap() at trap+0x238
--- trap (number -1769486) ---
Bad frame pointer: 0xd5d7bffe
0:
ddb ps
   PID   PPID   PGRPUID  S   FLAGS  WAIT  COMMAND
  1637   3787   3787 70  3   0  biowait   named
  3787   5609   3787  0  30x80  netio named
  5609  23313  29579  0  30x80  piperdnamed
 23313  24670  29579  0  30x88  pause sh
 24670  29579  29579  0  30x88  pause sh
   279  26321  26321 73  3   0  biowait   syslogd
 26321  1  26321  0  30x80  netio syslogd
 29579  1  29579  0  30x88  pause sh
13  0  0  0  30x100200  aiodoned  aiodoned
12  0  0  0  30x100200  syncerupdate
11  0  0  0  30x100200  

Re: Apache's FAQ.html

[Mikolaj Kucharski]

On Fri, Jan 27, 2012 at 08:42:08AM +, Jason McIntyre wrote:
 hmm. there are more problems than this:
 
 - some of the pages in htdocs refer to a FAQ page at
   http://httpd.apache.org/docs/misc/FAQ.html. the link doesn;t work
   though.
 
 - misc/index.html refers to FAQ.html, the file you propose to remove.
 
 your intention seems correct though (i agree misc/FAQ.html looks
 broken).
 
 the reference in misc/index.html can be trivially fixed. i've no idea
 about the apache link though.
 
 jmc


I would also change link in ssl faq too:

Index: mod/mod_ssl/ssl_faq.html
===
RCS file: /cvs/src/usr.sbin/httpd/htdocs/manual/mod/mod_ssl/ssl_faq.html,v
retrieving revision 1.12
diff -u -r1.12 ssl_faq.html
--- mod/mod_ssl/ssl_faq.html18 Jun 2008 05:42:29 -  1.12
+++ mod/mod_ssl/ssl_faq.html27 Jan 2012 18:34:58 -
@@ -500,7 +500,7 @@
 are done with full year value instead of abbreviating to two digits.
 p
 Additionally according to a a
-href=http://www.apache.org/docs/misc/FAQ.html#year2000;Year 2000
+href=../../misc/FAQ.html#year2000Year 2000
 statement/a from the Apache Group, the Apache webserver is Year 2000
 compliant, too. But whether OpenSSL or the underlaying Operating System
 (either a Unix or Win32 platform) is Year 2000 compliant is a different




Looking at latest online FAQ for Apache 1.3 it looks even worse than
in-tree manual/FAQ.html

http://httpd.apache.org/docs/1.3/misc/FAQ.html

so I'm also not sure what's the option here.

-- 
best regards
q#

Re: Apache's FAQ.html

[Mikolaj Kucharski]

On Thu, Sep 15, 2011 at 11:51:00PM +0100, Mikolaj Kucharski wrote:
 Hi,
 
 Looking at the /usr/src/usr.sbin/httpd/htdocs/manual files I think file
 misc/FAQ.html should be overwritten by FAQ.html and then FAQ.html can be
 removed as nothing reference to that location. All links point to
 misc/FAQ.html.

In other words, this is what I mean:


$ cd /usr/src/usr.sbin/httpd/htdocs/manual
$ ls -l FAQ.html misc/FAQ.html
-rw-r--r--  1 build  build  153831 Jun 21  2009 FAQ.html
-rw-r--r--  1 build  build5463 Jun 21  2009 misc/FAQ.html
$ cat FAQ.html  misc/FAQ.html
$ rm -f FAQ.html
$ cvs rm FAQ.html


I didn't run make release with above, so not sure are there any
additional changes needed, by looking at the source of html files only
misc/FAQ.html file is used, other one isn't. However as you can see
above from the sizes, content of misc/FAQ.html is much smaller and if
you open that file in the browser, file looks broken.


PS. Please CC me in any replies.

-- 
best regards
q#

ncurses 5.7 update and define USE_SIGWINCH

[Mikolaj Kucharski]

Hi,

I was troubleshooting issue with window resize in net/ekg. In the past
it was working properly, but stopped. I figured out that breakage
happened between OpenBSD 4.6 and 4.7, when ncurses was updated.

I've recompiled ncurses with -DTRACE and and used it with very simple
program to test (see screensize.c under url mentioned below).

Comparing tracing of above code from OpenBSD 4.6 and OpenBSD 4.9
(-current) show that in the latest code function _nc_update_screensize()
is not executed in doupdate() (lib/libcurses/tty/tty_update.c). It is
executed however in OpenBSD 4.6. Reason is that USE_SIZECHANGE is
defined as 0 (zero) bacause of following code (curses.priv.h from 4.9):


#if HAVE_SIZECHANGE  USE_SIGWINCH  defined(SIGWINCH)
#define USE_SIZECHANGE 1
#else
#define USE_SIZECHANGE 0
#undef USE_SIGWINCH
#define USE_SIGWINCH 0
#endif


HAVE_SIZECHANGE is defined to 1 in OpenBSD 4.6 and 4.9, but USE_SIGWINCH
is not defined to 1. In OpenBSD 4.6 only HAVE_SIZECHANGE was checked to
set USE_SIZECHANGE to 1 (curses.priv.h from 4.6):


#if HAVE_SIZECHANGE
#define USE_SIZECHANGE 1
#else
#undef USE_SIGWINCH
#endif


Is there any reason why USE_SIGWINCH is not defined to 1 in current
OpenBSD (in ncurses_cfg.h)?


You can find my debugging files under following location:

 http://www1.kucharski.name/pub/ekg/

- screensize.c -- code to test the issue

- patches for libcurses which enable tracing and adds some additional
logging

- trace logs, so you can compare how ncurses on OpenBSD 4.6, unpatched
4.9 and with USE_SIGWINCH defined to 1 on 4.9 behave



-- 
best regards
q#

Re: ncurses 5.7 update and define USE_SIGWINCH

[Mikolaj Kucharski]

On Fri, Apr 22, 2011 at 01:33:14PM -0600, Theo de Raadt wrote:
  Is there any reason why USE_SIGWINCH is not defined to 1 in current
  OpenBSD (in ncurses_cfg.h)?
 
 Yes.
 
 Libraries should not use signals like that.
 
 We could start off by pointing out that the code it enables in curses
 is NOT SAFE.

Ok. What options do I have to fix net/ekg? Compare it with mutt or irssi
(they aren't affected by USE_SIGWINCH define) and leave libcurses as it
is or is there any way to bring back behaviour from ncurses 5.2 to 5.7?

-- 
best regards
q#

Re: ncurses 5.7 update and define USE_SIGWINCH

[Mikolaj Kucharski]

On Fri, Apr 22, 2011 at 10:08:59PM +0200, Matthias Kilian wrote:
 Hi,
 
 On Fri, Apr 22, 2011 at 08:55:35PM +0100, Mikolaj Kucharski wrote:
  Ok. What options do I have to fix net/ekg? Compare it with mutt or irssi
  (they aren't affected by USE_SIGWINCH define) and leave libcurses as it
  is or is there any way to bring back behaviour from ncurses 5.2 to 5.7?
 
 You may have a look at www/snownews, from 2008-09-12 22:07. Martynas
 and I hacked a little bit (installing our own signal handler). This
 had been merged upstream to snownews later.

Problem is net/ekg handles SIGWINCH in the same way like www/snownews.
That means both are affected by USE_SIGWINCH define to 0. Start
snownews, open help window and resize the screen.

-- 
best regards
q#