A small fix for if_em.c, if_igb.c, if_ixgbe.c

[Randall Stewart]

Michael:


Looking at this patch (as I apply it to my world), I think 
you can just take the 

> -		if ((err = igb_xmit(txr, &next)) != 0) {
> +		if (igb_xmit(txr, &next) != 0) {

Type lines

and leave the 

return(err)

since err will get set to 0 by the drbr_enqueue() and return the proper response to the
transport above sending the packet.

R
On Nov 29, 2013, at 12:24 PM, Michael Tuexen wrote:

> Dear all,
> 
> ifnet(9) says regarding if_transmit():
> 
> Transmit a packet on an interface or queue it if the interface is
> in use.  This function will return ENOBUFS if the devices software
> and hardware queues are both full.
> 
> The drivers for em, igb and ixgbe might also return an error even
> in the case the packet was enqueued. The attached patches fix this
> issue.
> 
> Any comments?
> 
> Jack: What do you think? Would you prefer to commit the fix if
> you think it is acceptable?
> 
> Best regards
> Michael
> 
> 
> [bsd5:~/head/sys/dev] tuexen% svn diff -x -p
> Index: e1000/if_em.c
> ===================================================================
> --- e1000/if_em.c	(revision 258746)
> +++ e1000/if_em.c	(working copy)
> @@ -930,7 +930,7 @@ em_mq_start_locked(struct ifnet *ifp, struct tx_ri
> 
> 	/* Process the queue */
> 	while ((next = drbr_peek(ifp, txr->br)) != NULL) {
> -		if ((err = em_xmit(txr, &next)) != 0) {
> +		if (em_xmit(txr, &next) != 0) {
> 			if (next == NULL)
> 				drbr_advance(ifp, txr->br);
> 			else 
> @@ -957,7 +957,7 @@ em_mq_start_locked(struct ifnet *ifp, struct tx_ri
> 		em_txeof(txr);
> 	if (txr->tx_avail < EM_MAX_SCATTER)
> 		ifp->if_drv_flags |= IFF_DRV_OACTIVE;
> -	return (err);
> +	return (0);
> }
> 
> /*
> Index: e1000/if_igb.c
> ===================================================================
> --- e1000/if_igb.c	(revision 258746)
> +++ e1000/if_igb.c	(working copy)
> @@ -192,7 +192,7 @@ static int	igb_suspend(device_t);
> static int	igb_resume(device_t);
> #ifndef IGB_LEGACY_TX
> static int	igb_mq_start(struct ifnet *, struct mbuf *);
> -static int	igb_mq_start_locked(struct ifnet *, struct tx_ring *);
> +static void	igb_mq_start_locked(struct ifnet *, struct tx_ring *);
> static void	igb_qflush(struct ifnet *);
> static void	igb_deferred_mq_start(void *, int);
> #else
> @@ -989,31 +989,31 @@ igb_mq_start(struct ifnet *ifp, struct mbuf *m)
> 	if (err)
> 		return (err);
> 	if (IGB_TX_TRYLOCK(txr)) {
> -		err = igb_mq_start_locked(ifp, txr);
> +		igb_mq_start_locked(ifp, txr);
> 		IGB_TX_UNLOCK(txr);
> 	} else
> 		taskqueue_enqueue(que->tq, &txr->txq_task);
> 
> -	return (err);
> +	return (0);
> }
> 
> -static int
> +static void
> igb_mq_start_locked(struct ifnet *ifp, struct tx_ring *txr)
> {
> 	struct adapter  *adapter = txr->adapter;
>         struct mbuf     *next;
> -        int             err = 0, enq = 0;
> +        int             enq = 0;
> 
> 	IGB_TX_LOCK_ASSERT(txr);
> 
> 	if (((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) ||
> 	    adapter->link_active == 0)
> -		return (ENETDOWN);
> +		return;
> 
> 
> 	/* Process the queue */
> 	while ((next = drbr_peek(ifp, txr->br)) != NULL) {
> -		if ((err = igb_xmit(txr, &next)) != 0) {
> +		if (igb_xmit(txr, &next) != 0) {
> 			if (next == NULL) {
> 				/* It was freed, move forward */
> 				drbr_advance(ifp, txr->br);
> @@ -1045,7 +1045,7 @@ igb_mq_start_locked(struct ifnet *ifp, struct tx_r
> 		igb_txeof(txr);
> 	if (txr->tx_avail <= IGB_MAX_SCATTER)
> 		txr->queue_status |= IGB_QUEUE_DEPLETED;
> -	return (err);
> +	return;
> }
> 
> /*
> Index: ixgbe/ixgbe.c
> ===================================================================
> --- ixgbe/ixgbe.c	(revision 258746)
> +++ ixgbe/ixgbe.c	(working copy)
> @@ -107,7 +107,7 @@ static void     ixgbe_start(struct ifnet *);
> static void     ixgbe_start_locked(struct tx_ring *, struct ifnet *);
> #else /* ! IXGBE_LEGACY_TX */
> static int	ixgbe_mq_start(struct ifnet *, struct mbuf *);
> -static int	ixgbe_mq_start_locked(struct ifnet *, struct tx_ring *);
> +static void	ixgbe_mq_start_locked(struct ifnet *, struct tx_ring *);
> static void	ixgbe_qflush(struct ifnet *);
> static void	ixgbe_deferred_mq_start(void *, int);
> #endif /* IXGBE_LEGACY_TX */
> @@ -831,35 +831,35 @@ ixgbe_mq_start(struct ifnet *ifp, struct mbuf *m)
> 	if (err)
> 		return (err);
> 	if (IXGBE_TX_TRYLOCK(txr)) {
> -		err = ixgbe_mq_start_locked(ifp, txr);
> +		ixgbe_mq_start_locked(ifp, txr);
> 		IXGBE_TX_UNLOCK(txr);
> 	} else
> 		taskqueue_enqueue(que->tq, &txr->txq_task);
> 
> -	return (err);
> +	return (0);
> }
> 
> -static int
> +static void
> ixgbe_mq_start_locked(struct ifnet *ifp, struct tx_ring *txr)
> {
> 	struct adapter  *adapter = txr->adapter;
>         struct mbuf     *next;
> -        int             enqueued = 0, err = 0;
> +        int             enqueued = 0;
> 
> 	if (((ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) ||
> 	    adapter->link_active == 0)
> -		return (ENETDOWN);
> +		return;
> 
> 	/* Process the queue */
> #if __FreeBSD_version < 901504
> 	next = drbr_dequeue(ifp, txr->br);
> 	while (next != NULL) {
> -		if ((err = ixgbe_xmit(txr, &next)) != 0) {
> +		if (ixgbe_xmit(txr, &next) != 0) {
> 			if (next != NULL)
> -				err = drbr_enqueue(ifp, txr->br, next);
> +				drbr_enqueue(ifp, txr->br, next);
> #else
> 	while ((next = drbr_peek(ifp, txr->br)) != NULL) {
> -		if ((err = ixgbe_xmit(txr, &next)) != 0) {
> +		if (ixgbe_xmit(txr, &next) != 0) {
> 			if (next == NULL) {
> 				drbr_advance(ifp, txr->br);
> 			} else {
> @@ -890,7 +890,7 @@ ixgbe_mq_start_locked(struct ifnet *ifp, struct tx
> 	if (txr->tx_avail < IXGBE_TX_CLEANUP_THRESHOLD)
> 		ixgbe_txeof(txr);
> 
> -	return (err);
> +	return;
> }
> 
> /*
> 
> _______________________________________________
> freebsd-net at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"
> 

------------------------------
Randall Stewart
803-317-4952 (cell)