Re: usb(4): use cacheable buffers for data transfers (massive speedup)

2020-03-31 Thread Jonathan Gray
On Wed, Apr 01, 2020 at 12:58:23PM +1100, Jonathan Gray wrote:
> On Wed, Mar 18, 2020 at 01:41:06PM +0100, Patrick Wildt wrote:
> > On Wed, Mar 18, 2020 at 11:22:40AM +0100, Patrick Wildt wrote:
> > > Hi,
> > > 
> > > I've spent a few days investigating why USB ethernet adapters are so
> > > horribly slow on my ARMs.  Using dt(4) I realized that it was spending
> > > most of its time in memcpy.  But, why?  As it turns out, all USB data
> > > buffers are mapped COHERENT, which on some/most ARMs means uncached.
> > > Using cached data buffers makes the performance rise from 20 mbit/s to
> > > 200 mbit/s.  Quite a difference.
> > > 
> > > sys/dev/usb/usb_mem.c:
> > >   error = bus_dmamem_map(tag, p->segs, p->nsegs, p->size,
> > >  >kaddr, BUS_DMA_NOWAIT|BUS_DMA_COHERENT);
> > > 
> > > On x86, COHERENT is essentially a no-op.  On ARM, it depends on the SoC.
> > > Some SoCs have cache-coherent USB controllers, some don't.  Mine does
> > > not, so mapping it COHERENT means uncached and thus slow.
> > > 
> > > Why do we do that?  Well, when the code was imported in 99, it was
> > > already there.  Since then we have gained infrastructure for DMA
> > > syncs in the USB stack, which I think are proper.
> > > 
> > > sys/dev/usb/usbdi.c - usbd_transfer() (before transfer)
> > > 
> > >   if (!usbd_xfer_isread(xfer)) {
> > >   if ((xfer->flags & USBD_NO_COPY) == 0)
> > >   memcpy(KERNADDR(>dmabuf, 0), xfer->buffer,
> > >   xfer->length);
> > >   usb_syncmem(>dmabuf, 0, xfer->length,
> > >   BUS_DMASYNC_PREWRITE);
> > >   } else
> > >   usb_syncmem(>dmabuf, 0, xfer->length,
> > >   BUS_DMASYNC_PREREAD);
> > >   err = pipe->methods->transfer(xfer);
> > > 
> > > sys/dev/usb/usbdi.c - usb_transfer_complete() (after transfer)
> > > 
> > >   if (xfer->actlen != 0) {
> > >   if (usbd_xfer_isread(xfer)) {
> > >   usb_syncmem(>dmabuf, 0, xfer->actlen,
> > >   BUS_DMASYNC_POSTREAD);
> > >   if (!(xfer->flags & USBD_NO_COPY))
> > >   memcpy(xfer->buffer, KERNADDR(>dmabuf, 0),
> > >   xfer->actlen);
> > >   } else
> > >   usb_syncmem(>dmabuf, 0, xfer->actlen,
> > >   BUS_DMASYNC_POSTWRITE);
> > >   }
> > > 
> > > We cannot just remove COHERENT, since some drivers, like ehci(4), use
> > > the same backend to allocate their rings.  And I can't vouch for those
> > > drivers' sanity.
> > > 
> > > As a first step, I would like to go ahead with another solution, which
> > > is based on a diff from Marius Strobl, who added those syncs in the
> > > first place.  Essentially it splits the memory handling into cacheable
> > > and non-cacheable blocks.  The USB data transfers and everyone who uses
> > > usbd_alloc_buffer() then use cacheable buffers, while code like ehci(4)
> > > still don't.  This is a bit of a safer approach imho, since we don't
> > > hurt the controller drivers, but speed up the data buffers.
> > > 
> > > Once we have verified that there are no regressions, we can adjust
> > > ehci(4) and the like, add proper syncs, make sure they still work as
> > > well as before, and maybe then back this out again.
> > > 
> > > Keep note that this is all a no-op on X86, but all the other archs will
> > > profit from this.
> > > 
> > > ok?
> > > 
> > > Patrick
> > 
> > Update diff with inverted logic.  kettenis@ argues that we should
> > invert the logic, and those who need COHERENT memory should ask
> > for that explicitly, since for bus_dmamem_map() it also needs to
> > be passed explicitly.  This also points out all those users that
> > use usb_allocmem() internally, where we might want to have a look
> > if COHERENT is actually needed or not, or if it can be refactored
> > in another way.
> 
> These commits broke usb on imx.6 with cubox:
> 
> imxehci0 at simplebus3
> usb0 at imxehci0: USB revision 2.0
> usb0: root hub problem
> imxehci1 at simplebus3
> usb1 at imxehci1: USB revision 2.0
> usb1: root hub problem
> "usbmisc" at simplebus3 not configured

pandaboard with omap4 (another cortex a9) also has broken usb with
the latest snapshot:

omehci0 at simplebus0
usb0 at omehci0: USB revision 2.0
usb0: root hub problem



Re: usb(4): use cacheable buffers for data transfers (massive speedup)

2020-03-31 Thread Jonathan Gray
On Wed, Mar 18, 2020 at 01:41:06PM +0100, Patrick Wildt wrote:
> On Wed, Mar 18, 2020 at 11:22:40AM +0100, Patrick Wildt wrote:
> > Hi,
> > 
> > I've spent a few days investigating why USB ethernet adapters are so
> > horribly slow on my ARMs.  Using dt(4) I realized that it was spending
> > most of its time in memcpy.  But, why?  As it turns out, all USB data
> > buffers are mapped COHERENT, which on some/most ARMs means uncached.
> > Using cached data buffers makes the performance rise from 20 mbit/s to
> > 200 mbit/s.  Quite a difference.
> > 
> > sys/dev/usb/usb_mem.c:
> > error = bus_dmamem_map(tag, p->segs, p->nsegs, p->size,
> >>kaddr, BUS_DMA_NOWAIT|BUS_DMA_COHERENT);
> > 
> > On x86, COHERENT is essentially a no-op.  On ARM, it depends on the SoC.
> > Some SoCs have cache-coherent USB controllers, some don't.  Mine does
> > not, so mapping it COHERENT means uncached and thus slow.
> > 
> > Why do we do that?  Well, when the code was imported in 99, it was
> > already there.  Since then we have gained infrastructure for DMA
> > syncs in the USB stack, which I think are proper.
> > 
> > sys/dev/usb/usbdi.c - usbd_transfer() (before transfer)
> > 
> > if (!usbd_xfer_isread(xfer)) {
> > if ((xfer->flags & USBD_NO_COPY) == 0)
> > memcpy(KERNADDR(>dmabuf, 0), xfer->buffer,
> > xfer->length);
> > usb_syncmem(>dmabuf, 0, xfer->length,
> > BUS_DMASYNC_PREWRITE);
> > } else
> > usb_syncmem(>dmabuf, 0, xfer->length,
> > BUS_DMASYNC_PREREAD);
> > err = pipe->methods->transfer(xfer);
> > 
> > sys/dev/usb/usbdi.c - usb_transfer_complete() (after transfer)
> > 
> > if (xfer->actlen != 0) {
> > if (usbd_xfer_isread(xfer)) {
> > usb_syncmem(>dmabuf, 0, xfer->actlen,
> > BUS_DMASYNC_POSTREAD);
> > if (!(xfer->flags & USBD_NO_COPY))
> > memcpy(xfer->buffer, KERNADDR(>dmabuf, 0),
> > xfer->actlen);
> > } else
> > usb_syncmem(>dmabuf, 0, xfer->actlen,
> > BUS_DMASYNC_POSTWRITE);
> > }
> > 
> > We cannot just remove COHERENT, since some drivers, like ehci(4), use
> > the same backend to allocate their rings.  And I can't vouch for those
> > drivers' sanity.
> > 
> > As a first step, I would like to go ahead with another solution, which
> > is based on a diff from Marius Strobl, who added those syncs in the
> > first place.  Essentially it splits the memory handling into cacheable
> > and non-cacheable blocks.  The USB data transfers and everyone who uses
> > usbd_alloc_buffer() then use cacheable buffers, while code like ehci(4)
> > still don't.  This is a bit of a safer approach imho, since we don't
> > hurt the controller drivers, but speed up the data buffers.
> > 
> > Once we have verified that there are no regressions, we can adjust
> > ehci(4) and the like, add proper syncs, make sure they still work as
> > well as before, and maybe then back this out again.
> > 
> > Keep note that this is all a no-op on X86, but all the other archs will
> > profit from this.
> > 
> > ok?
> > 
> > Patrick
> 
> Update diff with inverted logic.  kettenis@ argues that we should
> invert the logic, and those who need COHERENT memory should ask
> for that explicitly, since for bus_dmamem_map() it also needs to
> be passed explicitly.  This also points out all those users that
> use usb_allocmem() internally, where we might want to have a look
> if COHERENT is actually needed or not, or if it can be refactored
> in another way.

These commits broke usb on imx.6 with cubox:

imxehci0 at simplebus3
usb0 at imxehci0: USB revision 2.0
usb0: root hub problem
imxehci1 at simplebus3
usb1 at imxehci1: USB revision 2.0
usb1: root hub problem
"usbmisc" at simplebus3 not configured

After reverting them I can use filesystems on usb again.

diff --git sys/dev/usb/dwc2/dwc2.c sys/dev/usb/dwc2/dwc2.c
index 6ca3cc658e5..6f035467213 100644
--- sys/dev/usb/dwc2/dwc2.c
+++ sys/dev/usb/dwc2/dwc2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dwc2.c,v 1.51 2020/03/21 12:08:31 patrick Exp $   */
+/* $OpenBSD: dwc2.c,v 1.49 2019/11/27 11:16:59 mpi Exp $   */
 /* $NetBSD: dwc2.c,v 1.32 2014/09/02 23:26:20 macallan Exp $   */
 
 /*-
@@ -474,7 +474,7 @@ dwc2_open(struct usbd_pipe *pipe)
case UE_CONTROL:
pipe->methods = _device_ctrl_methods;
err = usb_allocmem(>sc_bus, sizeof(usb_device_request_t),
-   0, USB_DMA_COHERENT, >req_dma);
+   0, >req_dma);
if (err)
return err;
break;
diff --git sys/dev/usb/dwc2/dwc2_hcd.c sys/dev/usb/dwc2/dwc2_hcd.c
index ab76de7d766..7e5c91481d5 100644
--- sys/dev/usb/dwc2/dwc2_hcd.c
+++ sys/dev/usb/dwc2/dwc2_hcd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: 

Re: Prevent memory corruption by pipex_timer()

2020-03-31 Thread Vitaliy Makkoveev
On Tue, Mar 31, 2020 at 06:15:46PM +0200, Martin Pieuchot wrote:
> On 31/03/20(Tue) 18:58, Vitaliy Makkoveev wrote:
> > On Tue, Mar 31, 2020 at 05:23:46PM +0200, Martin Pieuchot wrote:
> > > On 31/03/20(Tue) 16:48, Vitaliy Makkoveev wrote:
> > > > I refactored pppx(4). The idea is to use pipex(4) as it was designed.
> > > > No one holds pipex_session outside pipex(4) so pipex_timer() calls are
> > > > safe. Unfortunately, this way gives some performance impact, because we
> > > > need to call pipex_lookup_by_session_id() in some places. This impact
> > > > will gone after pipex_session becames safely shared between multiple
> > > > holders and this is my next goal.
> > > 
> > > I'd be more confident if we could go with the one-line change that you
> > > submitted in the first mail of this thread without the debugging
> > > messages.
> > > 
> > > Mixing bug-fixes (or features) and refactoring is not a great
> > > development practise as it might hide the point of the change and
> > > introduce or expose new bugs. 
> > >
> > I changed my original patch. Since npppd(8) ignores ioctl() errors and
> > client will be connected without pppx(4) interface creation I decide to
> > lie npppd(8).
> 
> Well better fix npppd(8), no?  Not crashing the kernel is priority 1.
I made patch for npppd(8) too. I include it to this email below, without
starting new thread, ok? If ioctl(PIPEXASESSION) failed and error !=
ENXIO it means that pipex is enabled and session creation failed so down
this connection.

> Then if the daemon has a bug, should the kernel work around it? 
In most common cases should :(

 cut begin

Index: usr.sbin/npppd/npppd/ppp.c
===
RCS file: /cvs/src/usr.sbin/npppd/npppd/ppp.c,v
retrieving revision 1.28
diff -u -p -r1.28 ppp.c
--- usr.sbin/npppd/npppd/ppp.c  27 Feb 2019 04:52:19 -  1.28
+++ usr.sbin/npppd/npppd/ppp.c  31 Mar 2020 19:59:21 -
@@ -1134,8 +1134,12 @@ ppp_on_network_pipex(npppd_ppp *_this)
(!MPPE_MUST_NEGO(_this) || _this->ccp.fsm.state == OPENED ||
_this->ccp.fsm.state == STOPPED)) {
/* IPCP is opened and MPPE is not required or MPPE is opened */
-   if (npppd_ppp_pipex_enable(_this->pppd, _this) != 0)
+   if (npppd_ppp_pipex_enable(_this->pppd, _this) != 0) {
ppp_log(_this, LOG_WARNING, "failed enable pipex: %m");
+   /* failed to create pipex session */
+   ppp_phy_downed(_this);
+   return;
+   }
ppp_log(_this, LOG_NOTICE, "Using pipex=%s",
(_this->pipex_enabled != 0)? "yes" : "no");
_this->pipex_started = 1;

 cut end

And patch for pppx()

 cut begin

Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.77
diff -u -p -r1.77 if_pppx.c
--- sys/net/if_pppx.c   26 Mar 2020 16:50:46 -  1.77
+++ sys/net/if_pppx.c   31 Mar 2020 20:00:33 -
@@ -665,6 +665,12 @@ pppx_add_session(struct pppx_dev *pxd, s
struct ifnet *over_ifp = NULL;
 #endif
 
+   /*
+* XXX: prevent pxi destruction by pipex_timer()
+*/
+   if (req->pr_timeout_sec != 0)
+   return (EINVAL);
+
switch (req->pr_protocol) {
 #ifdef PIPEX_PPPOE
case PIPEX_PROTO_PPPOE:

 cut end



Re: Prevent memory corruption by pipex_timer()

2020-03-31 Thread Martin Pieuchot
On 31/03/20(Tue) 18:58, Vitaliy Makkoveev wrote:
> On Tue, Mar 31, 2020 at 05:23:46PM +0200, Martin Pieuchot wrote:
> > On 31/03/20(Tue) 16:48, Vitaliy Makkoveev wrote:
> > > I refactored pppx(4). The idea is to use pipex(4) as it was designed.
> > > No one holds pipex_session outside pipex(4) so pipex_timer() calls are
> > > safe. Unfortunately, this way gives some performance impact, because we
> > > need to call pipex_lookup_by_session_id() in some places. This impact
> > > will gone after pipex_session becames safely shared between multiple
> > > holders and this is my next goal.
> > 
> > I'd be more confident if we could go with the one-line change that you
> > submitted in the first mail of this thread without the debugging
> > messages.
> > 
> > Mixing bug-fixes (or features) and refactoring is not a great
> > development practise as it might hide the point of the change and
> > introduce or expose new bugs. 
> >
> I changed my original patch. Since npppd(8) ignores ioctl() errors and
> client will be connected without pppx(4) interface creation I decide to
> lie npppd(8).

Well better fix npppd(8), no?  Not crashing the kernel is priority 1.
Then if the daemon has a bug, should the kernel work around it? 

> Index: net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.77
> diff -u -p -r1.77 if_pppx.c
> --- net/if_pppx.c 26 Mar 2020 16:50:46 -  1.77
> +++ net/if_pppx.c 31 Mar 2020 15:48:26 -
> @@ -665,6 +665,13 @@ pppx_add_session(struct pppx_dev *pxd, s
>   struct ifnet *over_ifp = NULL;
>  #endif
>  
> + /*
> +  * XXX: prevent pxi destruction by pipex_timer()
> +  * npppd(8) ignores pppx_add_session() errors so fake it
> +  */
> + if (req->pr_timeout_sec != 0)
> + req->pr_timeout_sec = 0;
> +
>   switch (req->pr_protocol) {
>  #ifdef PIPEX_PPPOE
>   case PIPEX_PROTO_PPPOE:
> 



Re: Prevent memory corruption by pipex_timer()

2020-03-31 Thread Vitaliy Makkoveev
On Tue, Mar 31, 2020 at 05:23:46PM +0200, Martin Pieuchot wrote:
> On 31/03/20(Tue) 16:48, Vitaliy Makkoveev wrote:
> > I refactored pppx(4). The idea is to use pipex(4) as it was designed.
> > No one holds pipex_session outside pipex(4) so pipex_timer() calls are
> > safe. Unfortunately, this way gives some performance impact, because we
> > need to call pipex_lookup_by_session_id() in some places. This impact
> > will gone after pipex_session becames safely shared between multiple
> > holders and this is my next goal.
> 
> I'd be more confident if we could go with the one-line change that you
> submitted in the first mail of this thread without the debugging
> messages.
> 
> Mixing bug-fixes (or features) and refactoring is not a great
> development practise as it might hide the point of the change and
> introduce or expose new bugs. 
>
I changed my original patch. Since npppd(8) ignores ioctl() errors and
client will be connected without pppx(4) interface creation I decide to
lie npppd(8).

Index: net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.77
diff -u -p -r1.77 if_pppx.c
--- net/if_pppx.c   26 Mar 2020 16:50:46 -  1.77
+++ net/if_pppx.c   31 Mar 2020 15:48:26 -
@@ -665,6 +665,13 @@ pppx_add_session(struct pppx_dev *pxd, s
struct ifnet *over_ifp = NULL;
 #endif
 
+   /*
+* XXX: prevent pxi destruction by pipex_timer()
+* npppd(8) ignores pppx_add_session() errors so fake it
+*/
+   if (req->pr_timeout_sec != 0)
+   req->pr_timeout_sec = 0;
+
switch (req->pr_protocol) {
 #ifdef PIPEX_PPPOE
case PIPEX_PROTO_PPPOE:



Re: Prevent memory corruption by pipex_timer()

2020-03-31 Thread Martin Pieuchot
On 31/03/20(Tue) 16:48, Vitaliy Makkoveev wrote:
> I refactored pppx(4). The idea is to use pipex(4) as it was designed.
> No one holds pipex_session outside pipex(4) so pipex_timer() calls are
> safe. Unfortunately, this way gives some performance impact, because we
> need to call pipex_lookup_by_session_id() in some places. This impact
> will gone after pipex_session becames safely shared between multiple
> holders and this is my next goal.

I'd be more confident if we could go with the one-line change that you
submitted in the first mail of this thread without the debugging
messages.

Mixing bug-fixes (or features) and refactoring is not a great
development practise as it might hide the point of the change and
introduce or expose new bugs. 



Re: Prevent memory corruption by pipex_timer()

2020-03-31 Thread Vitaliy Makkoveev
I refactored pppx(4). The idea is to use pipex(4) as it was designed.
No one holds pipex_session outside pipex(4) so pipex_timer() calls are
safe. Unfortunately, this way gives some performance impact, because we
need to call pipex_lookup_by_session_id() in some places. This impact
will gone after pipex_session becames safely shared between multiple
holders and this is my next goal.

Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.77
diff -u -p -r1.77 if_pppx.c
--- sys/net/if_pppx.c   26 Mar 2020 16:50:46 -  1.77
+++ sys/net/if_pppx.c   31 Mar 2020 13:28:54 -
@@ -155,8 +155,8 @@ struct pppx_if {
int pxi_unit;
struct ifnetpxi_if;
struct pppx_dev *pxi_dev;
-   struct pipex_sessionpxi_session;
struct pipex_iface_context  pxi_ifcontext;
+   int pxi_ppp_id;
 };
 
 static inline int
@@ -178,7 +178,8 @@ int pppx_del_session(struct pppx_dev *,
 intpppx_set_session_descr(struct pppx_dev *,
struct pipex_session_descr_req *);
 
-void   pppx_if_destroy(struct pppx_dev *, struct pppx_if *);
+intpppx_if_destroy(struct pppx_dev *, struct pppx_if *,
+   struct pipex_session_close_req *);
 void   pppx_if_start(struct ifnet *);
 intpppx_if_output(struct ifnet *, struct mbuf *,
struct sockaddr *, struct rtentry *);
@@ -587,7 +588,7 @@ pppxclose(dev_t dev, int flags, int mode
/* XXX */
NET_LOCK();
while ((pxi = LIST_FIRST(>pxd_pxis)))
-   pppx_if_destroy(pxd, pxi);
+   pppx_if_destroy(pxd, pxi, NULL);
NET_UNLOCK();
 
LIST_REMOVE(pxd, pxd_entry);
@@ -655,160 +656,14 @@ int
 pppx_add_session(struct pppx_dev *pxd, struct pipex_session_req *req)
 {
struct pppx_if *pxi;
-   struct pipex_session *session;
-   struct pipex_hash_head *chain;
struct ifnet *ifp;
int unit, error = 0;
struct in_ifaddr *ia;
struct sockaddr_in ifaddr;
-#ifdef PIPEX_PPPOE
-   struct ifnet *over_ifp = NULL;
-#endif
-
-   switch (req->pr_protocol) {
-#ifdef PIPEX_PPPOE
-   case PIPEX_PROTO_PPPOE:
-   over_ifp = ifunit(req->pr_proto.pppoe.over_ifname);
-   if (over_ifp == NULL)
-   return (EINVAL);
-   if (req->pr_peer_address.ss_family != AF_UNSPEC)
-   return (EINVAL);
-   break;
-#endif
-#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
-   case PIPEX_PROTO_PPTP:
-   case PIPEX_PROTO_L2TP:
-   switch (req->pr_peer_address.ss_family) {
-   case AF_INET:
-   if (req->pr_peer_address.ss_len != sizeof(struct 
sockaddr_in))
-   return (EINVAL);
-   break;
-#ifdef INET6
-   case AF_INET6:
-   if (req->pr_peer_address.ss_len != sizeof(struct 
sockaddr_in6))
-   return (EINVAL);
-   break;
-#endif
-   default:
-   return (EPROTONOSUPPORT);
-   }
-   if (req->pr_peer_address.ss_family !=
-   req->pr_local_address.ss_family ||
-   req->pr_peer_address.ss_len !=
-   req->pr_local_address.ss_len)
-   return (EINVAL);
-   break;
-#endif /* defined(PIPEX_PPTP) || defined(PIPEX_L2TP) */
-   default:
-   return (EPROTONOSUPPORT);
-   }
 
pxi = pool_get(pppx_if_pl, PR_WAITOK | PR_ZERO);
-   if (pxi == NULL)
-   return (ENOMEM);
-
-   session = >pxi_session;
ifp = >pxi_if;
 
-   /* fake a pipex interface context */
-   session->pipex_iface = >pxi_ifcontext;
-   session->pipex_iface->ifnet_this = ifp;
-   session->pipex_iface->pipexmode = PIPEX_ENABLED;
-
-   /* setup session */
-   session->state = PIPEX_STATE_OPENED;
-   session->protocol = req->pr_protocol;
-   session->session_id = req->pr_session_id;
-   session->peer_session_id = req->pr_peer_session_id;
-   session->peer_mru = req->pr_peer_mru;
-   session->timeout_sec = req->pr_timeout_sec;
-   session->ppp_flags = req->pr_ppp_flags;
-   session->ppp_id = req->pr_ppp_id;
-
-   session->ip_forward = 1;
-
-   session->ip_address.sin_family = AF_INET;
-   session->ip_address.sin_len = sizeof(struct sockaddr_in);
-   session->ip_address.sin_addr = req->pr_ip_address;
-
-   session->ip_netmask.sin_family = AF_INET;
-   session->ip_netmask.sin_len = sizeof(struct sockaddr_in);
-   session->ip_netmask.sin_addr = req->pr_ip_netmask;
-
-   if (session->ip_netmask.sin_addr.s_addr == 0L)
-   session->ip_netmask.sin_addr.s_addr = 

Re: PATCH: configurable tiling in cwm(1)

2020-03-31 Thread Okan Demirmen
On Fri 2020.03.27 at 00:30 +, Uwe Werler wrote:
> Hello @tech,
> 
> with this diff https://marc.info/?l=openbsd-tech=149192690925713=2 the
> behaviour of cwm changed so vtile and htile always use 50% of the screen.
> 
> This is a reasonable default but sometimes I want to have the master windows
> be bigger.
> 
> Inspired by this patch https://marc.info/?l=openbsd-tech=154887686615696=2
> I suggest the following diff.
> 
> This adds two new config options "htile percent" and "vtile percent" which
> define how much of the screen the current windows should occupy as the master
> window. If set to "0" then the old behaviour is restored where the current
> vertical or horizontal size of the window defines the size of the master
> window. If unset the current behaviour is unchaged that means the master
> windows uses half of the screen size.
> 
> For example in .cwmrc:
> 
> htile 66
> vtile 0
> 
> For window-htile the master window will occupy 66% of the screen high or for
> window-vtile the current window width defines the screen width of the master
> window.
> 
> Any thoughts?

I'm not a tiling user, but I have no real objection. Not a fan of the config
option, but I can't think of anything else off the top of my head. That said,
outside of a spacing fixes, I did re-word the cwmrc.5 bits as such (if it reads
well to other tile users):

 htile percent
 Set the percentage of screen the master window should occupy
 after calling window-htile.  If set to 0, the horizontal size of
 the master window will remain unchanged.  The default is 50.

 vtile percent
 Set the percentage of screen the master window should occupy
 after calling window-vtile.  If set to 0, the vertical size of
 the master window will remain unchanged.  The default is 50.


Index: calmwm.h
===
RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v
retrieving revision 1.374
diff -u -p -r1.374 calmwm.h
--- calmwm.h24 Mar 2020 14:47:29 -  1.374
+++ calmwm.h31 Mar 2020 12:04:21 -
@@ -291,6 +291,8 @@ struct conf {
int  bwidth;
int  mamount;
int  snapdist;
+   int  htile;
+   int  vtile;
struct gap   gap;
char*color[CWM_COLOR_NITEMS];
char*font;
Index: client.c
===
RCS file: /home/open/cvs/xenocara/app/cwm/client.c,v
retrieving revision 1.262
diff -u -p -r1.262 client.c
--- client.c24 Mar 2020 14:47:29 -  1.262
+++ client.c31 Mar 2020 12:05:51 -
@@ -940,7 +940,8 @@ client_htile(struct client_ctx *cc)
cc->geom.x = area.x;
cc->geom.y = area.y;
cc->geom.w = area.w - (cc->bwidth * 2);
-   cc->geom.h = (area.h - (cc->bwidth * 2)) / 2;
+   if (Conf.htile > 0)
+   cc->geom.h = ((area.h - (cc->bwidth * 2)) * Conf.htile) / 100;
client_resize(cc, 1);
client_ptr_warp(cc);
 
@@ -1007,7 +1008,8 @@ client_vtile(struct client_ctx *cc)
cc->flags &= ~CLIENT_VMAXIMIZED;
cc->geom.x = area.x;
cc->geom.y = area.y;
-   cc->geom.w = (area.w - (cc->bwidth * 2)) / 2;
+   if (Conf.vtile > 0)
+   cc->geom.w = ((area.w - (cc->bwidth * 2)) * Conf.vtile) / 100;
cc->geom.h = area.h - (cc->bwidth * 2);
client_resize(cc, 1);
client_ptr_warp(cc);
Index: conf.c
===
RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v
retrieving revision 1.251
diff -u -p -r1.251 conf.c
--- conf.c  27 Feb 2020 14:56:39 -  1.251
+++ conf.c  31 Mar 2020 12:04:21 -
@@ -281,6 +281,8 @@ conf_init(struct conf *c)
c->stickygroups = 0;
c->bwidth = 1;
c->mamount = 1;
+   c->htile = 50;
+   c->vtile = 50;
c->snapdist = 0;
c->ngroups = 0;
c->nameqlen = 5;
Index: cwmrc.5
===
RCS file: /home/open/cvs/xenocara/app/cwm/cwmrc.5,v
retrieving revision 1.75
diff -u -p -r1.75 cwmrc.5
--- cwmrc.5 13 Mar 2020 20:50:07 -  1.75
+++ cwmrc.5 31 Mar 2020 12:31:46 -
@@ -183,6 +183,13 @@ This
 can be used for applications such as
 .Xr xclock 1 ,
 where the user may wish to remain visible.
+.It Ic htile Ar percent
+Set the percentage of screen the master window should occupy
+after calling
+.Ic window-htile .
+If set to 0, the horizontal size of the master window will
+remain unchanged.
+The default is 50.
 .It Ic ignore Ar windowname
 Ignore, and do not warp to, windows with the name
 .Ar windowname
@@ -216,6 +223,13 @@ A special
 keyword
 .Dq all
 can be used to unbind all buttons.
+.It Ic vtile Ar percent
+Set the percentage of screen 

[patch] sys/queue.h

2020-03-31 Thread Martin
Hi there!

This changes the comment in sys/queue.h a little bit. I interpreted the
comment to describe not what generally is possible, but rather what
macros are implemented. While changing this also add that elements after
an existing element can be removed (SIMPLQ_REMOVE_AFTER).

I also noticed that the XSIMPLEQ* macros are not documented in the
queue.3 page. From what I gathered from the mailing list is, that these
were added for in kernel use. Is there a specific reason for not
mentioning them in queue.3? If no, would you be open to a diff?

Best,

Martin

Index: queue.h
===
RCS file: /cvs/src/sys/sys/queue.h,v
retrieving revision 1.45
diff -u -p -r1.45 queue.h
--- queue.h 12 Jul 2018 14:22:54 -  1.45
+++ queue.h 31 Mar 2020 10:24:16 -
@@ -62,9 +62,9 @@
  * A simple queue is headed by a pair of pointers, one to the head of the
  * list and the other to the tail of the list. The elements are singly
  * linked to save space, so elements can only be removed from the
- * head of the list. New elements can be added to the list before or after
- * an existing element, at the head of the list, or at the end of the
- * list. A simple queue may only be traversed in the forward direction.
+ * head of the list or after an existing element. New elements can be added to
+ * the list after an existing element, at the head of the list, or at the end
+ * of the list. A simple queue may only be traversed in the forward direction.
  *
  * A tail queue is headed by a pair of pointers, one to the head of the
  * list and the other to the tail of the list. The elements are doubly



Re: [PATCH] gostr341001: support unwrapped private keys support

2020-03-31 Thread Dmitry Baryshkov
Hello,

вт, 31 мар. 2020 г. в 06:20, Kinichiro Inoguchi :
>
> Hi,
>
> Where can we see the specifcation for these 3 different format, wrapped in 
> OCTET STRING, INTEGER and unwrapped but masked ?
> I tried to find but couldn't.

There is no English specification for GOST PKCS8 files yet,
unfortunately. You can find similar pieces of code in OpenSSL's GOST
engine (https://github.com/gost-engine/engine/blob/master/gost_ameth.c#L347)
and in GnuTLS 
(https://gitlab.com/gnutls/gnutls/-/blob/master/lib/x509/privkey_pkcs8.c#L1159).

-- 
With best wishes
Dmitry



Re: MSI-X for ix(4) again

2020-03-31 Thread Martin Pieuchot
On 26/03/20(Thu) 12:13, Martin Pieuchot wrote:
> Previous diff had a bug in the interrupt handler refactoring that showed
> up on i386.  The version below has been tested on i386 and doesn't seem
> to cause any regression.
> 
> Here's the original explanation, diff below has msix toggle to "off":
> 
> > Diff below allows ix(4) to establish two MSI-X handlers.  Multiqueue
> > support is intentionally not included in this diff.
> > 
> > One handler is for the single queue (index ":0") and one for the
> > link interrupt:
> > 
> >   # vmstat -i |grep ix0
> >   irq114/ix0:0 73178178 3758
> >   irq115/ix0  20
> >
> > A per-driver toggle allows to switch MSI-X support on/off.  My plan is
> > to commit with the switch "off" for the moment.  Switching it to "on"
> > should be better done in the beginning of a release cycle.  However it
> > is "on" in the diff below for testing purposes.
> 
> I appreciate more tests and oks :)

Updated diff that only unable interrupt moderation on the link interrupt
if MSI-X is used.  Without this change the !msix path was receiving less
packets as found by Hrvoje.

This has been tested on both i386 and amd64, I'm quite confident, any
ok?

Index: dev/pci/if_ix.c
===
RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
retrieving revision 1.163
diff -u -p -r1.163 if_ix.c
--- dev/pci/if_ix.c 25 Mar 2020 17:20:46 -  1.163
+++ dev/pci/if_ix.c 30 Mar 2020 12:55:04 -
@@ -114,6 +114,8 @@ int ixgbe_media_change(struct ifnet *);
 void   ixgbe_identify_hardware(struct ix_softc *);
 intixgbe_allocate_pci_resources(struct ix_softc *);
 intixgbe_allocate_legacy(struct ix_softc *);
+intixgbe_allocate_msix(struct ix_softc *);
+intixgbe_setup_msix(struct ix_softc *);
 intixgbe_allocate_queues(struct ix_softc *);
 void   ixgbe_free_pci_resources(struct ix_softc *);
 void   ixgbe_local_timer(void *);
@@ -140,6 +142,7 @@ voidixgbe_initialize_rss_mapping(struct
 intixgbe_rxfill(struct rx_ring *);
 void   ixgbe_rxrefill(void *);
 
+intixgbe_intr(struct ix_softc *sc);
 void   ixgbe_enable_intr(struct ix_softc *);
 void   ixgbe_disable_intr(struct ix_softc *);
 void   ixgbe_update_stats_counters(struct ix_softc *);
@@ -172,11 +175,16 @@ void  ixgbe_handle_msf(struct ix_softc *)
 void   ixgbe_handle_phy(struct ix_softc *);
 
 /* Legacy (single vector interrupt handler */
-intixgbe_intr(void *);
+intixgbe_legacy_intr(void *);
 void   ixgbe_enable_queue(struct ix_softc *, uint32_t);
+void   ixgbe_enable_queues(struct ix_softc *);
 void   ixgbe_disable_queue(struct ix_softc *, uint32_t);
 void   ixgbe_rearm_queue(struct ix_softc *, uint32_t);
 
+/* MSI-X (multiple vectors interrupt handlers)  */
+intixgbe_link_intr(void *);
+intixgbe_queue_intr(void *);
+
 /*
  *  OpenBSD Device Interface Entry Points
  */
@@ -190,6 +198,7 @@ struct cfattach ix_ca = {
 };
 
 int ixgbe_smart_speed = ixgbe_smart_speed_on;
+int ixgbe_enable_msix = 0;
 
 /*
  *  Device identification routine
@@ -292,7 +301,10 @@ ixgbe_attach(struct device *parent, stru
bcopy(sc->hw.mac.addr, sc->arpcom.ac_enaddr,
IXGBE_ETH_LENGTH_OF_ADDRESS);
 
-   error = ixgbe_allocate_legacy(sc);
+   if (sc->msix > 1)
+   error = ixgbe_allocate_msix(sc);
+   else
+   error = ixgbe_allocate_legacy(sc);
if (error)
goto err_late;
 
@@ -524,6 +536,7 @@ ixgbe_ioctl(struct ifnet * ifp, u_long c
ixgbe_disable_intr(sc);
ixgbe_iff(sc);
ixgbe_enable_intr(sc);
+   ixgbe_enable_queues(sc);
}
error = 0;
}
@@ -819,6 +832,12 @@ ixgbe_init(void *arg)
itr |= IXGBE_EITR_LLI_MOD | IXGBE_EITR_CNT_WDIS;
IXGBE_WRITE_REG(>hw, IXGBE_EITR(0), itr);
 
+   if (sc->msix > 1) {
+   /* Set moderation on the Link interrupt */
+   IXGBE_WRITE_REG(>hw, IXGBE_EITR(sc->linkvec),
+   IXGBE_LINK_ITR);
+   }
+
/* Enable power to the phy */
if (sc->hw.phy.ops.set_phy_power)
sc->hw.phy.ops.set_phy_power(>hw, TRUE);
@@ -834,6 +853,7 @@ ixgbe_init(void *arg)
 
/* And now turn on interrupts */
ixgbe_enable_intr(sc);
+   ixgbe_enable_queues(sc);
 
/* Now inform the stack we're ready */
ifp->if_flags |= IFF_RUNNING;
@@ -964,6 +984,16 @@ ixgbe_enable_queue(struct ix_softc *sc, 
 }
 
 void
+ixgbe_enable_queues(struct ix_softc *sc)
+{
+   struct ix_queue *que;
+   int i;
+
+   for (i = 0, que = sc->queues; i < sc->num_queues; i++, que++)
+