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 B82DBA0C56;
	Wed,  1 Sep 2021 21:07:35 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 8538D40DF7;
	Wed,  1 Sep 2021 21:07:35 +0200 (CEST)
Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com
 [209.85.218.50]) by mails.dpdk.org (Postfix) with ESMTP id 2BC0140DDE
 for <dev@dpdk.org>; Wed,  1 Sep 2021 21:07:34 +0200 (CEST)
Received: by mail-ej1-f50.google.com with SMTP id a25so1354354ejv.6
 for <dev@dpdk.org>; Wed, 01 Sep 2021 12:07:34 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;
 h=mime-version:references:in-reply-to:from:date:message-id:subject:to
 :cc; bh=BfL52HCbd2haqPQvJnq2COg5Oa+pNxv7/T2XPblDhqI=;
 b=Z8b/eg/NPyxBXGDgaDxDrimrVDM88MnluiknUdFDdPXC1cSu7mzTtJ01ogiOMND/J4
 GJTS8kFPbyHk/npxoDo5yQDHzzmrDt/+ruvEYP+725h6Yk9TLicAZbg9htosEYBW5ze3
 xwQdfkoTf678fmBJbHC2bgVnEYv2DkrtOZcrtU+M4eCgyqeFMP8ONFg+wAU5+ut34Ar8
 M6orvvIbned1REpvGzJqx26/T1kTMtGuNL+dmZvird9bDWXfRRYsxVTqJLc+En7s+p/G
 QEJoedbXNrR4YTvvV6pNR8c9dx8PVmdSHLhu9OzlNaSv+xliFzOhn9HByI134Kwm7PpS
 uyFQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=BfL52HCbd2haqPQvJnq2COg5Oa+pNxv7/T2XPblDhqI=;
 b=lGiF27wjRew4xF4l2vEIFaWorReKg85VE/gHaV0kac9oWzpLZrgo9K/yODjPRzLv3o
 qRIFzdBpI4xSftMX4wSvjfCJTjS8NjFwzDKOTGnUMzsYZs4o2zfJlNYPV7PyR5wtNf7X
 GzC8VnLBXQnpm0tIcgG1gFOEn2CK4T/iTqiCAmfyI01T0geT33NAb7rfBbK6HSyPFniS
 8SJmiq6C4cP1KyWEUUTNxgDaZJHIY4UrXabdZMLPWEcSQ4OV28zyks61aoan16q3lkKM
 MrRrDTkEUhc8bOdt41BhzKL1vsFedeDumht/3qVi4k93eX6y5xUv+OeysrOoE/5vByDM
 WZ0g==
X-Gm-Message-State: AOAM5306yW/EaijfG6tQf7naBetUfQ1hmP4k4I6kDojqZ9S1dSz1iv4J
 vBR33GKL2YBBVw2gH5v5N00ORXE783bmOBsPq1g=
X-Google-Smtp-Source: ABdhPJzc3Cy/tZA2f3m1wmNUwR9d43Dr8WInJTLimrctBwmf/zPg5PgIMZzn8lnEKgXd8qCIJQdBBpHSj1r6TGxkZVE=
X-Received: by 2002:a17:906:1191:: with SMTP id
 n17mr1112568eja.244.1630523253833; 
 Wed, 01 Sep 2021 12:07:33 -0700 (PDT)
MIME-Version: 1.0
References: <1629463607-76292-1-git-send-email-tudor.cornea@gmail.com>
 <1483bf7f-09ec-edd5-1fc1-4b4c81206ae2@intel.com>
In-Reply-To: <1483bf7f-09ec-edd5-1fc1-4b4c81206ae2@intel.com>
From: Tudor Cornea <tudor.cornea@gmail.com>
Date: Wed, 1 Sep 2021 22:07:22 +0300
Message-ID: <CAOuQ8vVQKNpf==9c95eF=oNuKTTxqW2ZaVgcNex9UQJ60oVCAA@mail.gmail.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: linville@tuxdriver.com, thomas@monjalon.net, dev@dpdk.org
Content-Type: text/plain; charset="UTF-8"
X-Content-Filtered-By: Mailman/MimeDel 2.1.29
Subject: Re: [dpdk-dev] [PATCH] net/af_packet: try to reinsert the stripped
 vlan tag
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
Sender: "dev" <dev-bounces@dpdk.org>

Indeed, the vlan insertion could be a costly operation. We should probably
do it only if the user specifically asks to have the vlan tag in the packet.
Otherwise, af_packet PMD users might pay a price in terms of performance
for something they didn't ask for.

I was thinking of avoiding having to change the application in order to
re-insert the vlan tag.
Doing this operation inside the PMD driver seemed like a good fit.

Looking at the netvsc driver (drivers/net/netvsc), the vlan insertion is
guarded by a check to hv->vlan_strip

  if (!hv->vlan_strip && rte_vlan_insert(&m)) {

hv->vlan_strip seems to be initialized in hn_dev_configure() in the
following way

 hv->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);

while 'hv' seems to be stored in rte_eth_dev->data->dev_private

I am thinking of doing something similar for the af_packet PMD.
The 'pmd_internals' structure could potentially hold a field, say
vlan_strip', which could be initialized if the application enables the
DEV_RX_OFFLOAD_VLAN_STRIP in rxmode->offloads

This way, I'm thinking that the application could potentially control the
effect of vlan stripping for the af_packet PMD, in an uniform way, similar
to other PMDs.
Would this be considered an acceptable solution ?




On Tue, 31 Aug 2021 at 18:31, Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 8/20/2021 1:46 PM, Tudor Cornea wrote:
> > The af_packet pmd driver binds to a raw socket and allows
> > sending and receiving of packets through the kernel.
> >
> > Since commit bcc6d47903 [1], the kernel strips the vlan tags early in
> > __netif_receive_skb_core(), so we receive untagged packets while
> > running with the af_packet pmd.
> >
> > Luckily for us, the skb vlan-related fields are still populated from the
> > stripped vlan tags, so we end up having all the information
> > that we need in the mbuf.
> >
> > We would like to have the the vlan tag inside the mbuf.
> > Let's take a shot at it by trying to reinsert the stripped vlan tag.
> >
>
> PMD already sets 'mbuf->vlan_tci' and 'PKT_RX_VLAN | PKT_RX_VLAN_STRIPPED'
> flags, so application can be aware of the vlan tag and can consume it.
>
> Inserting the vlan tag back to packet is costly, what is the motivation to
> do so?
>
> > As a side note, something similar was done for the netvsc pmd.
> >
> > [1]
> https://github.com/torvalds/linux/commit/bcc6d47903612c3861201cc3a866fb604f26b8b2
> >
> > Signed-off-by: Tudor Cornea <tudor.cornea@gmail.com>
> > ---
> >  drivers/net/af_packet/rte_eth_af_packet.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/af_packet/rte_eth_af_packet.c
> b/drivers/net/af_packet/rte_eth_af_packet.c
> > index b73b211..d116583 100644
> > --- a/drivers/net/af_packet/rte_eth_af_packet.c
> > +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> > @@ -148,6 +148,10 @@ eth_af_packet_rx(void *queue, struct rte_mbuf
> **bufs, uint16_t nb_pkts)
> >               if (ppd->tp_status & TP_STATUS_VLAN_VALID) {
> >                       mbuf->vlan_tci = ppd->tp_vlan_tci;
> >                       mbuf->ol_flags |= (PKT_RX_VLAN |
> PKT_RX_VLAN_STRIPPED);
> > +
> > +                     /* the kernel always strips the vlan tag, try to
> reinsert it */
> > +                     if (rte_vlan_insert(&mbuf))
> > +                             PMD_LOG(ERR, "Failed to reinsert vlan
> tag");
> >               }
> >
> >               /* release incoming frame and advance ring buffer */
> >
>
>