From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 682BB4262C; Mon, 25 Sep 2023 12:30:46 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9BB3C40C35; Mon, 25 Sep 2023 12:30:45 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 1B7A940A79 for ; Mon, 25 Sep 2023 12:30:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695637842; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qcup5ZS4uz9NFlyK/llz9L8O6KWAVOJaePUi83FhJ7U=; b=cvZyNxzTOyzrKG/UZ4FiOC/eKYcnd8cAZ28myg0vAFnkp7pG4v7PmHVJDc6Us35OlUCgV1 wFHF6NZAuxcgaK17rOpvd8W5e5El0hbxt1tokozCgEqcBxkLxIsx3RmJKY43xEQ0ZDCmXn YT3YjhKWh1ff0N+vqyuaIVADIOCq8LQ= Received: from mail-lj1-f199.google.com (mail-lj1-f199.google.com [209.85.208.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-475-qY3eki5BOumX7oHUda2i7Q-1; Mon, 25 Sep 2023 06:30:40 -0400 X-MC-Unique: qY3eki5BOumX7oHUda2i7Q-1 Received: by mail-lj1-f199.google.com with SMTP id 38308e7fff4ca-2bfdcba1144so84918851fa.1 for ; Mon, 25 Sep 2023 03:30:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695637839; x=1696242639; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=qcup5ZS4uz9NFlyK/llz9L8O6KWAVOJaePUi83FhJ7U=; b=wOs+r+Fv4fZ6kK6XA6y6Q3Un4kwHEocdtgG+YwoU5Enxp+mtDNzPXMYbDsGdS3Iqxc InUfX7JIOmSKbq+Wz4K86rwezJq+KpWOop7uXpvyxTZxsIqA/ptakIJTjQ2CHQKh1WXf 2RnQi3IUeY5Q16zZ3E3HuKZ+eOhRqVwV5r3ETh0+4aVZOrFQCdK6RC+LamHyXuPe6E7b 3L9FRVDXIVdskOZIYUuTxdSuUjjZbh2RRvxYn7TymbUhTt/43g5jOupqByYh9k94SpnW JYmZGjqlup6g1MbBmErJO9SIfDGCHzsduw6LqJByMCPy8JM3vrM7OM9aHyDVl4pttwsR 16Gw== X-Gm-Message-State: AOJu0YxF0IrFgzJq/6b/6XNELcF7u7SGy2cHkT2zwsDf6h/xuyAuhlaL RDMzHFF5gYK2IyK5F7TgOSMvq8FuHSgEjesW7IP7Qc5XEx3M1REuLEWWfE9GULrVu+3TUPIuVHR Hzgzhdg/THRMj0PZIeNk= X-Received: by 2002:a2e:9dd7:0:b0:2b9:e53f:e201 with SMTP id x23-20020a2e9dd7000000b002b9e53fe201mr5450970ljj.31.1695637839428; Mon, 25 Sep 2023 03:30:39 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGOvQ+kKAQbwwkSEYtx89uCZRfRlWj5HfLzjL2E6uCzm3qnnT6Z3X590lqr6aMgfGAawMbGRiToXMQ8SNOEqmM= X-Received: by 2002:a2e:9dd7:0:b0:2b9:e53f:e201 with SMTP id x23-20020a2e9dd7000000b002b9e53fe201mr5450949ljj.31.1695637839115; Mon, 25 Sep 2023 03:30:39 -0700 (PDT) MIME-Version: 1.0 References: <20230919140430.3251493-1-david.marchand@redhat.com> <20230919140430.3251493-2-david.marchand@redhat.com> In-Reply-To: From: David Marchand Date: Mon, 25 Sep 2023 12:30:27 +0200 Message-ID: Subject: Re: [PATCH 2/2] net/ice: fix TSO with big segments To: "Zhang, Qi Z" Cc: "dev@dpdk.org" , "ktraynor@redhat.com" , "mkp@redhat.com" , "dexia.li@jaguarmicro.com" , "stable@dpdk.org" , "Yang, Qiming" X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, Sep 21, 2023 at 12:43=E2=80=AFPM Zhang, Qi Z = wrote: > > > > > -----Original Message----- > > From: David Marchand > > Sent: Thursday, September 21, 2023 1:55 PM > > To: Zhang, Qi Z > > Cc: dev@dpdk.org; ktraynor@redhat.com; mkp@redhat.com; > > dexia.li@jaguarmicro.com; stable@dpdk.org; Yang, Qiming > > ; Kevin Liu > > Subject: Re: [PATCH 2/2] net/ice: fix TSO with big segments > > > > On Thu, Sep 21, 2023 at 7:48=E2=80=AFAM Zhang, Qi Z wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: David Marchand > > > > Sent: Tuesday, September 19, 2023 10:05 PM > > > > To: dev@dpdk.org > > > > Cc: ktraynor@redhat.com; mkp@redhat.com; dexia.li@jaguarmicro.com; > > > > stable@dpdk.org; Yang, Qiming ; Zhang, Qi Z > > > > ; Kevin Liu > > > > Subject: [PATCH 2/2] net/ice: fix TSO with big segments > > > > > > > > Packets to be segmented with TSO are usually larger than MTU. > > > > Plus, a single segment for the whole packet may be used: in OVS > > > > case, an external rte_malloc'd buffer is used for packets received > > > > from vhost-user ports. > > > > > > > > Before this fix, TSO packets were dropped by net/ice with the > > > > following > > > > message: > > > > 2023-09-18T13:34:31.064Z|00020|dpdk(pmd- > > > > c31/id:22)|ERR|ice_prep_pkts(): > > > > INVALID mbuf: bad data_len=3D[2962] > > > > > > > > Remove the check on data_len. > > > > > > > > Besides, logging an error level message in a datapath function may > > > > slow down the whole application. It is better not to log anything. > > > > > > > > Fixes: ccf33dccf7aa ("net/ice: check illegal packet sizes") > > > > Cc: stable@dpdk.org > > > > > > > > Signed-off-by: David Marchand > > > > --- > > > > Note: there may be some followup patch later, as some additional > > > > check has been added in ice_prep_pkts. > > > > For context, see: > > > > > > http://inbox.dpdk.org/dev/CAJFAV8yOa3ShkVdEXHfnmOEmUTwV3e75Bu9U3 > > > > OqpNc5usTt3Rw@mail.gmail.com/T/#u > > > > > > > > --- > > > > drivers/net/ice/ice_rxtx.c | 8 +------- > > > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.= c > > > > index > > > > 64c4486b4b..80c4284200 100644 > > > > --- a/drivers/net/ice/ice_rxtx.c > > > > +++ b/drivers/net/ice/ice_rxtx.c > > > > @@ -3685,9 +3685,6 @@ ice_prep_pkts(__rte_unused void *tx_queue, > > > > struct rte_mbuf **tx_pkts, > > > > int i, ret; > > > > uint64_t ol_flags; > > > > struct rte_mbuf *m; > > > > - struct ice_tx_queue *txq =3D tx_queue; > > > > - struct rte_eth_dev *dev =3D &rte_eth_devices[txq->port_id]; > > > > - uint16_t max_frame_size =3D dev->data->mtu + ICE_ETH_OVERHEAD= ; > > > > > > > > for (i =3D 0; i < nb_pkts; i++) { > > > > m =3D tx_pkts[i]; > > > > @@ -3704,11 +3701,8 @@ ice_prep_pkts(__rte_unused void *tx_queue, > > > > struct rte_mbuf **tx_pkts, > > > > return i; > > > > } > > > > > > > > - /* check the data_len in mbuf */ > > > > - if (m->data_len < ICE_TX_MIN_PKT_LEN || > > > > - m->data_len > max_frame_size) { > > > > + if (m->pkt_len < ICE_TX_MIN_PKT_LEN) { > > > > > > +1 > > > > > > > rte_errno =3D EINVAL; > > > > - PMD_DRV_LOG(ERR, "INVALID mbuf: bad > > > > data_len=3D[%hu]", m->data_len); > > > > > > is it still worth to keep a debug level log here ? and it's better to= unify the > > logging method in the same function. > > > > Logging data_len is incorrect. > > > > There are no log in other drivers. > > > > If anything, the logging may happen in the application invoking > > rte_eth_tx_prepare. > > > > I am against keeping those logs. > > > I'm still hesitant to remove these logs until we find a way to provide eq= uivalent diagnostic information for users, because similar request comes d= irectly from some of our customers. > > There could be several options to consider, such as counting the errors a= nd reporting them in xstats or introducing devargs for on purpose diagnosti= c routine with log printing. This check indicates a programmatic error, in a datapath function. Keeping some log here while it could be triggered with packets is scary. Thinking about some xstats, what makes this check on the min packet length different from other checks in this helper? If we added a xstats for this check, we would have a super specialised counter for only this driver; And nobody would be able to make some sense of it without reading this driver code. --=20 David Marchand