From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 53E9E43E88;
	Tue, 16 Apr 2024 22:09:16 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id BF70F40268;
	Tue, 16 Apr 2024 22:09:15 +0200 (CEST)
Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com
 [209.85.214.178])
 by mails.dpdk.org (Postfix) with ESMTP id AD1C840262
 for <dev@dpdk.org>; Tue, 16 Apr 2024 22:09:13 +0200 (CEST)
Received: by mail-pl1-f178.google.com with SMTP id
 d9443c01a7336-1e651a9f3ffso16275695ad.1
 for <dev@dpdk.org>; Tue, 16 Apr 2024 13:09:13 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1713298153;
 x=1713902953; darn=dpdk.org; 
 h=content-transfer-encoding:mime-version:references:in-reply-to
 :message-id:subject:cc:to:from:date:from:to:cc:subject:date
 :message-id:reply-to;
 bh=QoGTGNpJH4jtqT8UhBgWmLvzYQedqevWKYQPED4VhWM=;
 b=cX5OwmB6iNDoE6LFVf87bOF/F5D+exNX9ezDy402MvL28cACUOQPUbLzOBWE1XPmFr
 ynYku2MN07GMusUG64+3PNeqxsZUU8IcUn32+mG96dGGUIV71Wefx8HuxlQ1r8zjawc9
 3O8yEBI8tbbIb6ITdT7neQBj2fU7MUr8WwDdVmM5QWZO9r0KUzu58M9Zn+woif1gOQZv
 VMpfH94tjguhq5xwtcwY0YHG89EYa4INgEZHlGkLV/t2Lx6I/COyBjwEIuvNZ6ZHZVye
 orvUEHwrlecK+OPd7xL/kqYl4wuf7B0IbxMZNrbI1qvX3A6ch9nPYEGXgnkGtEfOkieD
 8QWQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1713298153; x=1713902953;
 h=content-transfer-encoding:mime-version:references:in-reply-to
 :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc
 :subject:date:message-id:reply-to;
 bh=QoGTGNpJH4jtqT8UhBgWmLvzYQedqevWKYQPED4VhWM=;
 b=IlBM/mKKH6qAQhdbox5Gm8+NF3r6Dup4mUzGk7ZWVpSpJzGqr6H+tMp3HF5XwKPcIG
 Fozi4Ie6QxxdAeT+haSlBkA68UZ2BAttsJAxM4GVlFbMwEMvmC9mQ9oGQodUgsDOExP8
 fZkeMxB4HMWmULfllqHBe71CU0LlS47symP3zh+onh4LbNsHA+TT7sJ9RzVJRcy7yjjX
 lcsWOYeqyi/RP8A60967Aanu2u6EUyNgmhDo7lVRpVqqp6Ic8MppNXZY8NrRzZ74lbjo
 9VSqLVbC0Y2VJoH5XdsJC1fNheQshfNGqcqAvL1wOLnR1PxZFRu5W07IAH77MxS4dexY
 odCQ==
X-Forwarded-Encrypted: i=1;
 AJvYcCX/TMz/azm9C7f7+xIbbkezRGaY1JEj+Mo6GmPvQjnCg8DNAOQpMr12uJUcAt1tAtGY8eNYlnzfH8VPR5U=
X-Gm-Message-State: AOJu0Yx9odRxJS0B7hDQJfyIZZFZZpXhHQwMxeJabB6wtz0Znh2yBoFP
 6hYg0Yupn0QOqQr1JHrSmmPx4/2a9iYgjWZPTpYhywM+45fOCTo0ZJ8Qtept88dewMxshziy8l/
 x
X-Google-Smtp-Source: AGHT+IFs/goSutyKFRwvPv5NF+oB74lkX7rAgQ2AE/00Evy1AFNApGxXJqL1EQU1Yk1Q5BgGptVprQ==
X-Received: by 2002:a17:902:ea0e:b0:1e2:a2ca:6366 with SMTP id
 s14-20020a170902ea0e00b001e2a2ca6366mr13392615plg.21.1713298152657; 
 Tue, 16 Apr 2024 13:09:12 -0700 (PDT)
Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226])
 by smtp.gmail.com with ESMTPSA id
 i11-20020a17090332cb00b001e268c9e38asm10125634plr.43.2024.04.16.13.09.11
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Tue, 16 Apr 2024 13:09:12 -0700 (PDT)
Date: Tue, 16 Apr 2024 13:09:10 -0700
From: Stephen Hemminger <stephen@networkplumber.org>
To: Morten =?UTF-8?B?QnLDuHJ1cA==?= <mb@smartsharesystems.com>
Cc: "Konstantin Ananyev" <konstantin.ananyev@huawei.com>, <dev@dpdk.org>
Subject: Re: [DPDK/ethdev Bug 1416] net/af_packet: tx_burst() can modify
 packets
Message-ID: <20240416130910.22d1013f@hermes.local>
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F3AA@smartserver.smartshare.dk>
References: <bug-1416-3@http.bugs.dpdk.org/>
 <20240416083846.08e8dcad@hermes.local>
 <2b4a3bac389141bbae7a2d0ec31df5c6@huawei.com>
 <98CBD80474FA8B44BF855DF32C47DC35E9F3A9@smartserver.smartshare.dk>
 <e1b534c426f74273b891ac96f2d85e8f@huawei.com>
 <98CBD80474FA8B44BF855DF32C47DC35E9F3AA@smartserver.smartshare.dk>
MIME-Version: 1.0
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Tue, 16 Apr 2024 20:11:05 +0200
Morten Br=C3=B8rup <mb@smartsharesystems.com> wrote:

> > > > > > static uint16_t
> > > > > > eth_af_packet_tx(void *queue, struct rte_mbuf **bufs, uint16_t =
=20
> > > > nb_pkts) =20
> > > > > > {
> > > > > >         ...
> > > > > >         for (i =3D 0; i < nb_pkts; i++) {
> > > > > >                 mbuf =3D *bufs++;
> > > > > >
> > > > > >                 ...
> > > > > >
> > > > > >                 /* insert vlan info if necessary */
> > > > > >                 if (mbuf->ol_flags & RTE_MBUF_F_TX_VLAN) {
> > > > > >                         if (rte_vlan_insert(&mbuf)) {
> > > > > >                                 rte_pktmbuf_free(mbuf);
> > > > > >                                 continue;
> > > > > >
> > > > > > AFAIU, it does copy of mbuf contents into pbuf anyway (just few=
 =20
> > line =20
> > > > below). =20
> > > > > > So the fix might be - simply insert VLAN tag at copying stage.
> > > > > > Feel free to correct me, if I missed something. =20
> > > > >
> > > > > vlan_insert will fail if the mbuf is has refcnt > 1.
> > > > >
> > > > > static inline int rte_vlan_insert(struct rte_mbuf **m)
> > > > > {
> > > > > 	struct rte_ether_hdr *oh, *nh;
> > > > > 	struct rte_vlan_hdr *vh;
> > > > >
> > > > > 	/* Can't insert header if mbuf is shared */
> > > > > 	if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
> > > > > 		return -EINVAL; =20
> > > >
> > > > You are right, I missed that.
> > > > Will close it then. =20
> > >
> > > Don't close, silent drop is also a bug.
> > >
> > > The VLAN tag could be insert when copying, as originally suggested. =
=20
> >=20
> > Agree, but to me that would be enhancement request, not a bug report. =
=20
>=20
> Hmm... there is still a bug, although slightly different:
>=20
> net/af_packet: tx_burst() silently drops packets with RTE_MBUF_F_TX_VLAN =
if mbuf is shared
>=20
> And the suggested fixes would fix this (other) bug.


In older DPDK, vlan_insert would try and clone the mbuf, but doing a rte_pk=
tmbuf_clone().
But that was buggy and removed by:

commit 15a74163b12ed9b8b980b1576bdd8de16d60612b
Author: Ferruh Yigit <ferruh.yigit@intel.com>
Date:   Tue Apr 16 16:51:26 2019 +0100

    net: forbid VLAN insert in shared mbuf
   =20
    The vlan_insert() is buggy when it tries to handle the shared mbufs,
    instead don't support inserting VLAN tag into shared mbufs and return
    an error for that case.
   =20
    Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
    Acked-by: Olivier Matz <olivier.matz@6wind.com>