From owner-freebsd-net@FreeBSD.ORG Mon Dec 2 21:44:49 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6609FCAF; Mon, 2 Dec 2013 21:44:49 +0000 (UTC) Received: from mail-n.franken.de (drew.ipv6.franken.de [IPv6:2001:638:a02:a001:20e:cff:fe4a:feaa]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 8E9831AD9; Mon, 2 Dec 2013 21:44:48 +0000 (UTC) Received: from [192.168.1.102] (p508F2CD2.dip0.t-ipconnect.de [80.143.44.210]) (Authenticated sender: macmic) by mail-n.franken.de (Postfix) with ESMTP id 9CC361C0C0693; Mon, 2 Dec 2013 22:44:45 +0100 (CET) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 6.6 \(1510\)) Subject: Re: A small fix for if_em.c, if_igb.c, if_ixgbe.c From: Michael Tuexen In-Reply-To: <92126112-73DA-42D3-A8CD-DBF5FB8F45E8@lakerest.net> Date: Mon, 2 Dec 2013 22:44:46 +0100 Content-Transfer-Encoding: quoted-printable Message-Id: References: <521B9C2A-EECC-4412-9F68-2235320EF324@lurchi.franken.de> <92126112-73DA-42D3-A8CD-DBF5FB8F45E8@lakerest.net> To: Randall Stewart X-Mailer: Apple Mail (2.1510) Cc: Jack F Vogel , "freebsd-net@freebsd.org list" X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Dec 2013 21:44:49 -0000 On Dec 2, 2013, at 10:06 PM, Randall Stewart wrote: > Michael: >=20 >=20 > Looking at this patch (as I apply it to my world), I think=20 > you can just take the=20 >=20 >> - if ((err =3D igb_xmit(txr, &next)) !=3D 0) { >> + if (igb_xmit(txr, &next) !=3D 0) { >=20 > Type lines >=20 > and leave the=20 >=20 > return(err) >=20 > 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 >=20 > R > On Nov 29, 2013, at 12:24 PM, Michael Tuexen wrote: >=20 >> Dear all, >>=20 >> ifnet(9) says regarding if_transmit(): >>=20 >> 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. >>=20 >> 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. >>=20 >> Any comments? >>=20 >> Jack: What do you think? Would you prefer to commit the fix if >> you think it is acceptable? >>=20 >> Best regards >> Michael >>=20 >>=20 >> [bsd5:~/head/sys/dev] tuexen% svn diff -x -p >> Index: e1000/if_em.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- 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 >>=20 >> /* Process the queue */ >> while ((next =3D drbr_peek(ifp, txr->br)) !=3D NULL) { >> - if ((err =3D em_xmit(txr, &next)) !=3D 0) { >> + if (em_xmit(txr, &next) !=3D 0) { >> if (next =3D=3D NULL) >> drbr_advance(ifp, txr->br); >> else=20 >> @@ -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 |=3D IFF_DRV_OACTIVE; >> - return (err); >> + return (0); >> } >>=20 >> /* >> Index: e1000/if_igb.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- 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 =3D igb_mq_start_locked(ifp, txr); >> + igb_mq_start_locked(ifp, txr); >> IGB_TX_UNLOCK(txr); >> } else >> taskqueue_enqueue(que->tq, &txr->txq_task); >>=20 >> - return (err); >> + return (0); >> } >>=20 >> -static int >> +static void >> igb_mq_start_locked(struct ifnet *ifp, struct tx_ring *txr) >> { >> struct adapter *adapter =3D txr->adapter; >> struct mbuf *next; >> - int err =3D 0, enq =3D 0; >> + int enq =3D 0; >>=20 >> IGB_TX_LOCK_ASSERT(txr); >>=20 >> if (((ifp->if_drv_flags & IFF_DRV_RUNNING) =3D=3D 0) || >> adapter->link_active =3D=3D 0) >> - return (ENETDOWN); >> + return; >>=20 >>=20 >> /* Process the queue */ >> while ((next =3D drbr_peek(ifp, txr->br)) !=3D NULL) { >> - if ((err =3D igb_xmit(txr, &next)) !=3D 0) { >> + if (igb_xmit(txr, &next) !=3D 0) { >> if (next =3D=3D 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 <=3D IGB_MAX_SCATTER) >> txr->queue_status |=3D IGB_QUEUE_DEPLETED; >> - return (err); >> + return; >> } >>=20 >> /* >> Index: ixgbe/ixgbe.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- 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 =3D ixgbe_mq_start_locked(ifp, txr); >> + ixgbe_mq_start_locked(ifp, txr); >> IXGBE_TX_UNLOCK(txr); >> } else >> taskqueue_enqueue(que->tq, &txr->txq_task); >>=20 >> - return (err); >> + return (0); >> } >>=20 >> -static int >> +static void >> ixgbe_mq_start_locked(struct ifnet *ifp, struct tx_ring *txr) >> { >> struct adapter *adapter =3D txr->adapter; >> struct mbuf *next; >> - int enqueued =3D 0, err =3D 0; >> + int enqueued =3D 0; >>=20 >> if (((ifp->if_drv_flags & IFF_DRV_RUNNING) =3D=3D 0) || >> adapter->link_active =3D=3D 0) >> - return (ENETDOWN); >> + return; >>=20 >> /* Process the queue */ >> #if __FreeBSD_version < 901504 >> next =3D drbr_dequeue(ifp, txr->br); >> while (next !=3D NULL) { >> - if ((err =3D ixgbe_xmit(txr, &next)) !=3D 0) { >> + if (ixgbe_xmit(txr, &next) !=3D 0) { >> if (next !=3D NULL) >> - err =3D drbr_enqueue(ifp, txr->br, = next); >> + drbr_enqueue(ifp, txr->br, next); >> #else >> while ((next =3D drbr_peek(ifp, txr->br)) !=3D NULL) { >> - if ((err =3D ixgbe_xmit(txr, &next)) !=3D 0) { >> + if (ixgbe_xmit(txr, &next) !=3D 0) { >> if (next =3D=3D 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); >>=20 >> - return (err); >> + return; >> } >>=20 >> /* >>=20 >> _______________________________________________ >> freebsd-net@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-net >> To unsubscribe, send any mail to = "freebsd-net-unsubscribe@freebsd.org" >>=20 >=20 > ------------------------------ > Randall Stewart > 803-317-4952 (cell) >=20 >=20