Re: svn commit: r304326 - head/sys/dev/usb/net

2016-08-18 Thread YongHyeon PYUN
On Thu, Aug 18, 2016 at 04:44:29PM +, Bjoern A. Zeeb wrote:
> On 18 Aug 2016, at 5:07, Pyun YongHyeon wrote:
> 
> >Author: yongari
> >Date: Thu Aug 18 05:07:02 2016
> >New Revision: 304326
> >URL: https://svnweb.freebsd.org/changeset/base/304326
> >

[...]

> >+struct axge_frame_txhdr {
> >+#if BYTE_ORDER == LITTLE_ENDIAN
> >+uint32_tlen;
> >+#define AXGE_TXLEN_MASK 0x0001
> >+#define AXGE_VLAN_INSERT0x2000
> >+#define AXGE_CSUM_DISABLE   0x8000
> >+uint32_tmss;
> >+#define AXGE_MSS_MASK   0x3FFF
> >+#define AXGE_PADDING0x80008000
> >+#define AXGE_VLAN_TAG_MASK  0x
> >+#else
> >+uint32_tmss;
> >+uint32_tlen;
> >+#endif
> >+} __packed;
> >+
> >+#define AXGE_TXBYTES(x) ((x) & AXGE_TXLEN_MASK)
> 
> 
> AXGE_TXLEN_MASK is only defined for LITTLE_ENDIAN and thus breaks builds 
> on others.
> 
> AXGE_CSUM_DISABLE as well ..
> 

Oops, fixed in r304439.
Thanks.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r304326 - head/sys/dev/usb/net

2016-08-18 Thread Bjoern A. Zeeb

On 18 Aug 2016, at 5:07, Pyun YongHyeon wrote:


Author: yongari
Date: Thu Aug 18 05:07:02 2016
New Revision: 304326
URL: https://svnweb.freebsd.org/changeset/base/304326

Log:
  Switch to TX header format rather than directly manipulating header
  structures.  This simplifies mbuf copy operation to USB buffers as
  well as improving readability.  The controller supports Microsoft
  LSOv1(aka TSO) but this change set does not include the support due
  to copying overhead to USB buffers and large amount of memory waste.

  Remove useless ZLP padding which seems to come from Linux.  Required
  bits the code tried to set was not copied into USB buffer so it had
  no effect.  Unlike Linux, FreeBSD USB stack automatically generates
  ZLP so no explicit padding is required in driver.[1]

  Micro-optimize updating IFCOUNTER_OPACKETS counter by moving it out
  of TX loop since updating counter is not cheap operation as it did
  long time ago and we already know how many number of packets were
  queued after exiting the loop.

  While here, fix a checksum offloading bug which will happen when
  upper stack computes checksum while H/W checksum offloading is
  active.  The controller should be notified to not recompute the
  checksum in this case.

  Reviewed by:  kevlo (initial version), hselasky
  Pointed out by:   hselasky [1]

Modified:
  head/sys/dev/usb/net/if_axge.c
  head/sys/dev/usb/net/if_axgereg.h

Modified: head/sys/dev/usb/net/if_axge.c
==
--- head/sys/dev/usb/net/if_axge.c  Thu Aug 18 04:25:17 2016
(r304325)
+++ head/sys/dev/usb/net/if_axge.c  Thu Aug 18 05:07:02 2016
(r304326)
@@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -144,8 +145,8 @@ static const struct usb_config axge_conf
.type = UE_BULK,
.endpoint = UE_ADDR_ANY,
.direction = UE_DIR_OUT,
-   .frames = 16,
-   .bufsize = 16 * MCLBYTES,
+   .frames = AXGE_N_FRAMES,
+   .bufsize = AXGE_N_FRAMES * MCLBYTES,
.flags = {.pipe_bof = 1,.force_short_xfer = 1,},
.callback = axge_bulk_write_callback,
.timeout = 1,   /* 10 seconds */
@@ -630,7 +631,7 @@ axge_bulk_write_callback(struct usb_xfer
struct ifnet *ifp;
struct usb_page_cache *pc;
struct mbuf *m;
-   uint32_t txhdr;
+   struct axge_frame_txhdr txhdr;
int nframes, pos;

sc = usbd_xfer_softc(xfer);
@@ -651,36 +652,25 @@ tr_setup:
return;
}

-   for (nframes = 0; nframes < 16 &&
+   for (nframes = 0; nframes < AXGE_N_FRAMES &&
!IFQ_DRV_IS_EMPTY(>if_snd); nframes++) {
IFQ_DRV_DEQUEUE(>if_snd, m);
if (m == NULL)
break;
usbd_xfer_set_frame_offset(xfer, nframes * MCLBYTES,
-   nframes);
-   pos = 0;
+   nframes);
pc = usbd_xfer_get_frame(xfer, nframes);
-   txhdr = htole32(m->m_pkthdr.len);
-   usbd_copy_in(pc, 0, , sizeof(txhdr));
-   txhdr = 0;
-   txhdr = htole32(txhdr);
-   usbd_copy_in(pc, 4, , sizeof(txhdr));
-   pos += 8;
+   txhdr.mss = 0;
+   txhdr.len = htole32(AXGE_TXBYTES(m->m_pkthdr.len));
+   if ((ifp->if_capenable & IFCAP_TXCSUM) != 0 &&
+   (m->m_pkthdr.csum_flags & AXGE_CSUM_FEATURES) == 0)
+   txhdr.len |= htole32(AXGE_CSUM_DISABLE);
+
+   pos = 0;
+   usbd_copy_in(pc, pos, , sizeof(txhdr));
+   pos += sizeof(txhdr);
usbd_m_copy_in(pc, pos, m, 0, m->m_pkthdr.len);
pos += m->m_pkthdr.len;
-   if ((pos % usbd_xfer_max_framelen(xfer)) == 0)
-   txhdr |= 0x80008000;
-
-   /*
-* XXX
-* Update TX packet counter here. This is not
-* correct way but it seems that there is no way
-* to know how many packets are sent at the end
-* of transfer because controller combines
-* multiple writes into single one if there is
-* room in TX buffer of controller.
-*/
-   if_inc_counter(ifp, IFCOUNTER_OPACKETS, 1);

/*
 * if there's a BPF listener, bounce a copy
@@ -694,6 +684,16 @@ tr_setup:
 

svn commit: r304326 - head/sys/dev/usb/net

2016-08-17 Thread Pyun YongHyeon
Author: yongari
Date: Thu Aug 18 05:07:02 2016
New Revision: 304326
URL: https://svnweb.freebsd.org/changeset/base/304326

Log:
  Switch to TX header format rather than directly manipulating header
  structures.  This simplifies mbuf copy operation to USB buffers as
  well as improving readability.  The controller supports Microsoft
  LSOv1(aka TSO) but this change set does not include the support due
  to copying overhead to USB buffers and large amount of memory waste.
  
  Remove useless ZLP padding which seems to come from Linux.  Required
  bits the code tried to set was not copied into USB buffer so it had
  no effect.  Unlike Linux, FreeBSD USB stack automatically generates
  ZLP so no explicit padding is required in driver.[1]
  
  Micro-optimize updating IFCOUNTER_OPACKETS counter by moving it out
  of TX loop since updating counter is not cheap operation as it did
  long time ago and we already know how many number of packets were
  queued after exiting the loop.
  
  While here, fix a checksum offloading bug which will happen when
  upper stack computes checksum while H/W checksum offloading is
  active.  The controller should be notified to not recompute the
  checksum in this case.
  
  Reviewed by:  kevlo (initial version), hselasky
  Pointed out by:   hselasky [1]

Modified:
  head/sys/dev/usb/net/if_axge.c
  head/sys/dev/usb/net/if_axgereg.h

Modified: head/sys/dev/usb/net/if_axge.c
==
--- head/sys/dev/usb/net/if_axge.c  Thu Aug 18 04:25:17 2016
(r304325)
+++ head/sys/dev/usb/net/if_axge.c  Thu Aug 18 05:07:02 2016
(r304326)
@@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -144,8 +145,8 @@ static const struct usb_config axge_conf
.type = UE_BULK,
.endpoint = UE_ADDR_ANY,
.direction = UE_DIR_OUT,
-   .frames = 16,
-   .bufsize = 16 * MCLBYTES,
+   .frames = AXGE_N_FRAMES,
+   .bufsize = AXGE_N_FRAMES * MCLBYTES,
.flags = {.pipe_bof = 1,.force_short_xfer = 1,},
.callback = axge_bulk_write_callback,
.timeout = 1,   /* 10 seconds */
@@ -630,7 +631,7 @@ axge_bulk_write_callback(struct usb_xfer
struct ifnet *ifp;
struct usb_page_cache *pc;
struct mbuf *m;
-   uint32_t txhdr;
+   struct axge_frame_txhdr txhdr;
int nframes, pos;
 
sc = usbd_xfer_softc(xfer);
@@ -651,36 +652,25 @@ tr_setup:
return;
}
 
-   for (nframes = 0; nframes < 16 &&
+   for (nframes = 0; nframes < AXGE_N_FRAMES &&
!IFQ_DRV_IS_EMPTY(>if_snd); nframes++) {
IFQ_DRV_DEQUEUE(>if_snd, m);
if (m == NULL)
break;
usbd_xfer_set_frame_offset(xfer, nframes * MCLBYTES,
-   nframes);
-   pos = 0;
+   nframes);
pc = usbd_xfer_get_frame(xfer, nframes);
-   txhdr = htole32(m->m_pkthdr.len);
-   usbd_copy_in(pc, 0, , sizeof(txhdr));
-   txhdr = 0;
-   txhdr = htole32(txhdr);
-   usbd_copy_in(pc, 4, , sizeof(txhdr));
-   pos += 8;
+   txhdr.mss = 0;
+   txhdr.len = htole32(AXGE_TXBYTES(m->m_pkthdr.len));
+   if ((ifp->if_capenable & IFCAP_TXCSUM) != 0 &&
+   (m->m_pkthdr.csum_flags & AXGE_CSUM_FEATURES) == 0)
+   txhdr.len |= htole32(AXGE_CSUM_DISABLE);
+
+   pos = 0;
+   usbd_copy_in(pc, pos, , sizeof(txhdr));
+   pos += sizeof(txhdr);
usbd_m_copy_in(pc, pos, m, 0, m->m_pkthdr.len);
pos += m->m_pkthdr.len;
-   if ((pos % usbd_xfer_max_framelen(xfer)) == 0)
-   txhdr |= 0x80008000;
-
-   /*
-* XXX
-* Update TX packet counter here. This is not
-* correct way but it seems that there is no way
-* to know how many packets are sent at the end
-* of transfer because controller combines
-* multiple writes into single one if there is
-* room in TX buffer of controller.
-*/
-   if_inc_counter(ifp, IFCOUNTER_OPACKETS, 1);
 
/*
 * if there's a BPF listener, bounce a copy
@@ -694,6 +684,16 @@ tr_setup: