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

[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 <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 {
>