Date: Thu, 18 Aug 2016 16:44:29 +0000 From: "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net> To: "Pyun YongHyeon" <yongari@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r304326 - head/sys/dev/usb/net Message-ID: <F05575AA-18B3-4ABD-A346-59C210D76A8A@lists.zabbadoz.net> In-Reply-To: <201608180507.u7I572ZI096519@repo.freebsd.org> References: <201608180507.u7I572ZI096519@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <sys/systm.h> > #include <sys/bus.h> > #include <sys/condvar.h> > +#include <sys/endian.h> > #include <sys/kernel.h> > #include <sys/lock.h> > #include <sys/module.h> > @@ -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 = 10000, /* 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(&ifp->if_snd); nframes++) { > IFQ_DRV_DEQUEUE(&ifp->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, &txhdr, sizeof(txhdr)); > - txhdr = 0; > - txhdr = htole32(txhdr); > - usbd_copy_in(pc, 4, &txhdr, 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, &txhdr, 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: > usbd_xfer_set_frame_len(xfer, nframes, pos); > } > if (nframes != 0) { > + /* > + * 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, nframes); > usbd_xfer_set_frames(xfer, nframes); > usbd_transfer_submit(xfer); > ifp->if_drv_flags |= IFF_DRV_OACTIVE; > > Modified: head/sys/dev/usb/net/if_axgereg.h > ============================================================================== > --- head/sys/dev/usb/net/if_axgereg.h Thu Aug 18 04:25:17 > 2016 (r304325) > +++ head/sys/dev/usb/net/if_axgereg.h Thu Aug 18 05:07:02 > 2016 (r304326) > @@ -159,6 +159,26 @@ enum { > AXGE_N_TRANSFER, > }; > > +#define AXGE_N_FRAMES 16 > + > +struct axge_frame_txhdr { > +#if BYTE_ORDER == LITTLE_ENDIAN > + uint32_t len; > +#define AXGE_TXLEN_MASK 0x0001FFFF > +#define AXGE_VLAN_INSERT 0x20000000 > +#define AXGE_CSUM_DISABLE 0x80000000 > + uint32_t mss; > +#define AXGE_MSS_MASK 0x00003FFF > +#define AXGE_PADDING 0x80008000 > +#define AXGE_VLAN_TAG_MASK 0xFFFF0000 > +#else > + uint32_t mss; > + uint32_t len; > +#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 .. > + > #define AXGE_PHY_ADDR 3 > > struct axge_softc { >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F05575AA-18B3-4ABD-A346-59C210D76A8A>