Re: Improve ure(4) performance

2020-08-03 Thread 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

2020-08-03 Thread Jonathon Fletcher 
On Thu, Jul 30, 2020 at 06:56:48PM +, Mikolaj Kucharski wrote:
> 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!

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.

Thanks,
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_MAX  2
> >  
> > -#defineURE_TX_LIST_CNT 8
> > +#defineURE_TX_LIST_CNT 1
> >  #defineURE_RX_LIST_CNT 1
> > -#defineURE_RX_BUF_ALIGNsizeof(uint64_t)
> > +#defineURE_TX_BUF_ALIGN4
> > +#defineURE_RX_BUF_ALIGN8
> >  
> > -#defineURE_TXBUFSZ 16384
> > -#defineURE_8152_RXBUFSZ16384
> > -#defineURE_8153_RXBUFSZ32768
> > +#defineURE_TX_BUFSZ16384
> > +#defineURE_8152_RX_BUFSZ   16384
> > +#defineURE_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.c10 Jul 2020 13:26:41 -  1.16
> > +++ sys/dev/usb/if_ure.c28 Jul 2020 22:56:29 -
> > @@ -117,11 +117,13 @@ void  ure_miibus_writereg(struct device 
> >  void   ure_lock_mii(struct ure_softc *);
> >  void   ure_unlock_mii(struct ure_softc *);
> >  
> > -inture_encap(struct ure_softc *, struct mbuf *);
> > +inture_encap_txpkt(struct mbuf *, char *, uint32_t);
> > +inture_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);
> > -inture_rx_list_init(struct ure_softc *);
> > -inture_tx_list_init(struct ure_softc *);
> > +inture_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

Re: Improve ure(4) performance

2020-07-30 Thread 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));
> +
> + f

Re: Improve ure(4) performance

2020-07-30 Thread Jonathon Fletcher 
On Mon, Jul 27, 2020 at 07:13:33PM +, Mikolaj Kucharski wrote:
> 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.

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.

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_MAX  2
 
-#defineURE_TX_LIST_CNT 8
+#defineURE_TX_LIST_CNT 1
 #defineURE_RX_LIST_CNT 1
-#defineURE_RX_BUF_ALIGNsizeof(uint64_t)
+#defineURE_TX_BUF_ALIGN4
+#defineURE_RX_BUF_ALIGN8
 
-#defineURE_TXBUFSZ 16384
-#defineURE_8152_RXBUFSZ16384
-#defineURE_8153_RXBUFSZ32768
+#defineURE_TX_BUFSZ16384
+#defineURE_8152_RX_BUFSZ   16384
+#defineURE_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.c10 Jul 2020 13:26:41 -  1.16
+++ sys/dev/usb/if_ure.c28 Jul 2020 22:56:29 -
@@ -117,11 +117,13 @@ void  ure_miibus_writereg(struct device 
 void   ure_lock_mii(struct ure_softc *);
 void   ure_unlock_mii(struct ure_softc *);
 
-inture_encap(struct ure_softc *, struct mbuf *);
+inture_encap_txpkt(struct mbuf *, char *, uint32_t);
+inture_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);
-inture_rx_list_init(struct ure_softc *);
-inture_tx_list_init(struct ure_softc *);
+inture_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));
+
+   for (i = 0; i < URE_TX_LIST_CNT; i++) {
+   c = &sc->ure_cdata.ure_tx_chain[i];
+   if (ml_len(&c->uc_mbuf_list) > 0) {
+   usbd_get

Re: Improve ure(4) performance

2020-07-27 Thread Jonathon Fletcher 
On Mon, Jul 27, 2020 at 07:13:33PM +, Mikolaj Kucharski wrote:
> 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.

Mikolaj,

Great, thank you. I have attached the patch below with the whitespace intact.

Thanks,
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 27 Jul 2020 17:10:09 -
@@ -494,28 +494,32 @@ struct ure_txpkt {
 #define URE_ENDPT_TX   1
 #define URE_ENDPT_MAX  2
 
-#defineURE_TX_LIST_CNT 8
+#defineURE_TX_LIST_CNT 1
 #defineURE_RX_LIST_CNT 1
-#defineURE_RX_BUF_ALIGNsizeof(uint64_t)
+#defineURE_TX_BUF_ALIGN4
+#defineURE_RX_BUF_ALIGN8
 
-#defineURE_TXBUFSZ 16384
-#defineURE_8152_RXBUFSZ16384
-#defineURE_8153_RXBUFSZ32768
+#defineURE_8152_TX_BUFSZ   16384
+#defineURE_8152_RX_BUFSZ   16384
+
+#defineURE_8153_TX_BUFSZ   16384
+#defineURE_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)  ure_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 +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.c10 Jul 2020 13:26:41 -  1.16
+++ sys/dev/usb/if_ure.c27 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 *);
 
-inture_encap(struct ure_softc *, struct mbuf *);
+inture_encap_txpkt(struct mbuf *, char *, uint32_t);
+inture_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);
-inture_rx_list_init(struct ure_softc *);
-inture_tx_list_init(struct ure_softc *);
+inture_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 = &sc->ure_cdata.ure_tx_chain[i];
+   if (ml_len(&c->uc_mbuf_list) > 0) {
+   usbd_get_xfer_status(c->uc_xfer, NULL, NULL, NULL,
+   &err);
+   ure_txeof(c->uc_xfer, c, err);
+   }
+   }
+
+   if (ifq_is_oactive(&ifp->if_snd))
+

Re: Improve ure(4) performance

2020-07-27 Thread 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

2020-07-27 Thread Jonathon Fletcher 


> On Jul 27, 2020, at 11:47 AM, Mikolaj Kucharski  
> wrote:
> 
> 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&m=159587449018248&q=mbox 
> 

I appear to have a mangled whitespace problem with my email.

It applies successfully when I copy / paste the patch out of the url above and 
apply it from /usr/src with patch -p0 -l .

Are you ok using patch's -l option for this patch?

Thanks,
Jonathon

Re: Improve ure(4) performance

2020-07-27 Thread 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&m=159587449018248&q=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 = &sc->ure_cdata.ure_tx_chain[i];
> + if (ml_len(&c->uc_mbuf_list) > 0) {
> + usbd_get_xfer_status(c->uc_xfer, NULL, NULL, NULL,
> +&err);
> + ure_txeof(c->uc_xfer, c, err);
> + }
> + }
> +
> + if (ifq_is_oactive(&ifp->if_snd))
> + ifq_restart(&ifp->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(&sc->ure_cdata.ure_tx_free);
> + for (i = 0; i < URE_TX_LIST_CNT; i++)
> + SLIST_INSERT_HEAD(&sc->ure_cdata.ure_tx_free,
> + &sc->ure_cdata.ure_tx_chain[i], ure_list);
> +
>

Re: Improve ure(4) performance

2020-07-27 Thread Jonathon Fletcher 
On Sun, Jul 26, 2020 at 10:12 PM Mikolaj Kucharski
 wrote:
>
> 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/
>

[snip]

> xhci0 at pci0 dev 20 function 0 "Intel 100 Series xHCI" rev 0x21: 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
> uhub1 at uhub0 port 3 configuration 1 interface 0 "VIA Labs, Inc. USB2.0 Hub" 
> rev 2.10/3.d3 addr 2
> uhub2 at uhub1 port 3 configuration 1 interface 0 "Terminus Technology USB 
> 2.0 Hub" rev 2.00/1.00 addr 3
> 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
> rgephy0 at ure0 phy 0: RTL8251 PHY, rev. 0

Thanks Mikolaj,

I believe that the intermittent watchdog messages on RTL8153 variants
are now resolved. The previous code defined 32kb buffer sizes for both
tx and rx for RTL8153. The rx buffer size does vary by device type but
the tx buffer size should be 16kb for all types.

The patch improves tx performance primarily by packing multiple
packets into the outgoing xfer buffer if / when this is possible. This
makes it possible to use the entire xfer buffer in a single tx xfer.
If there are enough packets in the outbound queue and the xfer buffer
is defined as larger than the device buffer then the device could be
sent a tx xfer too large for it - eventually resulting in the watchdog
routine getting called.

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.

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 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 *, c

Re: Improve ure(4) performance

2020-07-26 Thread 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: 
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_CLE

Re: Improve ure(4) performance

2020-07-26 Thread Jonathon Fletcher 



> On Jul 26, 2020, at 1:24 PM, Mikolaj Kucharski  wrote:
> 
> 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.

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.

Thanks,
Jonathon

Re: Improve ure(4) performance

2020-07-26 Thread 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 = &sc->ure_cdata.ure_tx_chain[i];
> + if (ml_len(&c->uc_mbuf_list) > 0) {
> + usbd_get_xfer_status(c->uc_xfer, NULL, NULL, NULL,
> + &err);
> + ure_txeof(c->uc_xfer, c, err);
> + }
> + }
> +
> + if (ifq_is_oactive(&ifp->if_snd))
> + ifq_restart(&ifp->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(&sc->ure_cdata.ure_tx_free);
> + for (i = 0; i < URE_TX_LIST_CNT; i++)
> + SLIST_INSERT_HEAD(&sc->ure_cdata.ure_tx_free,
> + &sc->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 = &sc->ure_cdata.rx_chain[i];
> + c = &sc->ure_cdata.ure_rx_chain[i];
>   usbd_setup_xfer(c->uc_xfer, sc->ure_ep[URE_ENDPT_RX],
> -