From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47]) by dpdk.org (Postfix) with ESMTP id 767F41B618 for ; Fri, 3 Nov 2017 11:00:17 +0100 (CET) Received: by mail-wm0-f47.google.com with SMTP id z3so304359wme.5 for ; Fri, 03 Nov 2017 03:00:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:mime-version; bh=1ujgcbBU0/USjwmPrm7CW5XcirnJaw4zW51opSeXMGg=; b=ftkjwPNikyEUyQaUpzjvMeN90oPs9gzHoow24L2HkQ6Gvgy0hJJne08ZgC/q/kxVUf wBTXAoWmsjFGDwH1/DZcYt3Y9AS7P9XnclSxa+1AjANk6lbneEyTS52aMhassWVdFHme 6Ea1Szj3lZN9bNEeE78nKE6kVkjRSUA5mWcJbu3B+kUK2t3wYzpkSoF0o0iVtz58nfwU c28VHGzdHZmWU7h5yEkyGHrUAVnYqtXAblJG8t2fgK742+UXRqrw7rWXYZhGqN7/X7si IN+kjc3ThzcQ1RMfRQg0mV/hXUUQ50XuMxCtwAznwiqdMnLW1uRQ+d7Jz0Xjhh3EyYwU 0DTQ== X-Gm-Message-State: AMCzsaVRgXnH3rfGI+nLgyAVI7chwj5PaP238shxpVKDhvj4Ltwmphwk II8YghH2kcEEvqx3913FZlE= X-Google-Smtp-Source: ABhQp+TknYQpyXTAbODMdY/smdX+Hus6kXYLUs2O99F0qx1RjstooIaUjyFmYRZMTnAgA9pIW2BCmQ== X-Received: by 10.28.220.132 with SMTP id t126mr4245155wmg.51.1509703216960; Fri, 03 Nov 2017 03:00:16 -0700 (PDT) Received: from localhost ([2a00:23c5:bef3:400:4a51:b7ff:fe0b:4749]) by smtp.gmail.com with ESMTPSA id e6sm12918263wrg.53.2017.11.03.03.00.15 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 03 Nov 2017 03:00:16 -0700 (PDT) Message-ID: <1509703215.9111.3.camel@debian.org> From: Luca Boccassi To: John Daley , johnda888@gmail.com Cc: stable@dpdk.org Date: Fri, 03 Nov 2017 10:00:15 +0000 In-Reply-To: <20171102015653.1783-1-johndale@cisco.com> References: <20171102015653.1783-1-johndale@cisco.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Evolution 3.22.6-1+deb9u1 Mime-Version: 1.0 Subject: Re: [dpdk-stable] [PATCH] net/enic: fix TSO for packets greater than 9208 bytes X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Nov 2017 10:00:17 -0000 On Wed, 2017-11-01 at 18:56 -0700, John Daley wrote: > A check was previously added to drop Tx packets greater than what the > Nic > is capable of sending since such packets can freeze the send queue. > The > check did not account for TSO packets however, so TSO was limited to > 9208 > bytes. >=20 > Check packet length only for non-TSO packets. Also insure that TSO > packet > segment size plus the headers do not exceed what the Nic is capable > of > since this also can freeze the send queue. >=20 > Use the PKT_TX_TCP_SEG ol_flag instead of m->tso_segsz which is the > preferred way to check for TSO. >=20 > Fixes: ed6e564c214e ("net/enic: fix memory leak with oversized Tx > packets") > Cc: stable@dpdk.org >=20 > Signed-off-by: John Daley > --- >=20 > Note that there is some more work to do on enic TSO- the header > length is > calculated by looking at the packet instead of just trusting mbuf tso > offload header lengths. The 'tx_oversized' stat is used for more than > just > oversized packets- it gets rolled into 'oerrors' so doesn't matter > but the > name should be changed. Some TSO tunneling support can be added for > newer > hardware. These changes will come in the next relase, but hope that > this > patch can be accepted in 17.11 because it solves existing customer > problem. >=20 > =C2=A0drivers/net/enic/enic_rxtx.c | 25 +++++++++++++++++++------ > =C2=A01 file changed, 19 insertions(+), 6 deletions(-) >=20 > diff --git a/drivers/net/enic/enic_rxtx.c > b/drivers/net/enic/enic_rxtx.c > index a39172f14..e938193b5 100644 > --- a/drivers/net/enic/enic_rxtx.c > +++ b/drivers/net/enic/enic_rxtx.c > @@ -546,12 +546,15 @@ uint16_t enic_xmit_pkts(void *tx_queue, struct > rte_mbuf **tx_pkts, > =C2=A0 uint64_t bus_addr; > =C2=A0 uint8_t offload_mode; > =C2=A0 uint16_t header_len; > + uint64_t tso; > + rte_atomic64_t *tx_oversized; > =C2=A0 > =C2=A0 enic_cleanup_wq(enic, wq); > =C2=A0 wq_desc_avail =3D vnic_wq_desc_avail(wq); > =C2=A0 head_idx =3D wq->head_idx; > =C2=A0 desc_count =3D wq->ring.desc_count; > =C2=A0 ol_flags_mask =3D PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM | > PKT_TX_L4_MASK; > + tx_oversized =3D &enic->soft_stats.tx_oversized; > =C2=A0 > =C2=A0 nb_pkts =3D RTE_MIN(nb_pkts, ENIC_TX_XMIT_MAX); > =C2=A0 > @@ -561,10 +564,12 @@ uint16_t enic_xmit_pkts(void *tx_queue, struct > rte_mbuf **tx_pkts, > =C2=A0 data_len =3D tx_pkt->data_len; > =C2=A0 ol_flags =3D tx_pkt->ol_flags; > =C2=A0 nb_segs =3D tx_pkt->nb_segs; > + tso =3D ol_flags & PKT_TX_TCP_SEG; > =C2=A0 > - if (pkt_len > ENIC_TX_MAX_PKT_SIZE) { > + /* drop packet if it's too big to send */ > + if (unlikely(!tso && (pkt_len > > ENIC_TX_MAX_PKT_SIZE))) { > =C2=A0 rte_pktmbuf_free(tx_pkt); > - rte_atomic64_inc(&enic- > >soft_stats.tx_oversized); > + rte_atomic64_inc(tx_oversized); > =C2=A0 continue; > =C2=A0 } > =C2=A0 > @@ -587,13 +592,21 @@ uint16_t enic_xmit_pkts(void *tx_queue, struct > rte_mbuf **tx_pkts, > =C2=A0 offload_mode =3D WQ_ENET_OFFLOAD_MODE_CSUM; > =C2=A0 header_len =3D 0; > =C2=A0 > - if (tx_pkt->tso_segsz) { > + if (tso) { > =C2=A0 header_len =3D tso_header_len(tx_pkt); > - if (header_len) { > - offload_mode =3D > WQ_ENET_OFFLOAD_MODE_TSO; > - mss =3D tx_pkt->tso_segsz; > + > + /* Drop if non-TCP packet or TSO seg size is > too big */ > + if (unlikely((header_len =3D=3D 0) || ((tx_pkt- > >tso_segsz + > + =C2=A0=C2=A0=C2=A0=C2=A0header_len) > ENIC_TX_MAX_PKT_SIZE))) { > + rte_pktmbuf_free(tx_pkt); > + rte_atomic64_inc(tx_oversized); > + continue; > =C2=A0 } > + > + offload_mode =3D WQ_ENET_OFFLOAD_MODE_TSO; > + mss =3D tx_pkt->tso_segsz; > =C2=A0 } > + > =C2=A0 if ((ol_flags & ol_flags_mask) && (header_len =3D=3D 0)) > { > =C2=A0 if (ol_flags & PKT_TX_IP_CKSUM) > =C2=A0 mss |=3D ENIC_CALC_IP_CKSUM; Hi, Has this, or a version of this, been accepted into dpdk/master? I did a quick search but couldn't find it. I tried to apply it to dpdk-stable/16.11 but the context is quite different so it doesn't apply. If you would like it for 16.11.4, after it's accepted in dpdk/master, please send a reworked version that can be applied to dpdk-stable/16.11. Thanks! --=20 Kind regards, Luca Boccassi