Date: Mon, 2 Dec 2013 16:06:30 -0500 From: Randall Stewart <rrs@lakerest.net> To: Michael Tuexen <Michael.Tuexen@lurchi.franken.de> Cc: Jack F Vogel <jfv@freebsd.org>, "freebsd-net@freebsd.org list" <freebsd-net@freebsd.org> Subject: Re: A small fix for if_em.c, if_igb.c, if_ixgbe.c Message-ID: <92126112-73DA-42D3-A8CD-DBF5FB8F45E8@lakerest.net> In-Reply-To: <521B9C2A-EECC-4412-9F68-2235320EF324@lurchi.franken.de> References: <521B9C2A-EECC-4412-9F68-2235320EF324@lurchi.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
Michael:
Looking at this patch (as I apply it to my world), I think=20
you can just take the=20
> - if ((err =3D igb_xmit(txr, &next)) !=3D 0) {
> + if (igb_xmit(txr, &next) !=3D 0) {
Type lines
and leave the=20
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,
>=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
------------------------------
Randall Stewart
803-317-4952 (cell)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?92126112-73DA-42D3-A8CD-DBF5FB8F45E8>