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

[Michael Tuexen]

On Dec 2, 2013, at 10:06 PM, Randall Stewart <rrs at lakerest.net> wrote:

> 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.
True. Just thought this is clearer... But the patch is not minimal, you are right.

Best regards
Michael
> 
> 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)
> 
>