All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	"Bie, Tiwei" <tiwei.bie@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
	"Mcnamara, John" <john.mcnamara@intel.com>,
	"olivier.matz@6wind.com" <olivier.matz@6wind.com>,
	"thomas.monjalon@6wind.com" <thomas.monjalon@6wind.com>,
	"Zhang, Helin" <helin.zhang@intel.com>,
	"Dai, Wei" <wei.dai@intel.com>,
	"Wang, Xiao W" <xiao.w.wang@intel.com>
Subject: Re: [PATCH v5 3/8] ethdev: reserve capability flags for PMD-specific API
Date: Thu, 5 Jan 2017 11:32:38 +0000	[thread overview]
Message-ID: <2601191342CEEE43887BDE71AB9772583F1006B7@irsmsx105.ger.corp.intel.com> (raw)
In-Reply-To: <20170105083322.GK12822@6wind.com>

Hi Adrien,

> 
> On Thu, Jan 05, 2017 at 07:56:08AM +0800, Tiwei Bie wrote:
> > On Thu, Jan 05, 2017 at 01:44:18AM +0800, Ananyev, Konstantin wrote:
> > [...]
> > > > >
> > > > > I understand that.
> > > > > My question was: suppose user would like to create a bonded device over 2 NICs.
> > > > > One of them is ixgbe, while other would be some other type.
> > > > > In future get_dev_info() for each of them might return DEV_RX_OFFLOAD_RESERVED_0  bit as set.
> > > > > But it would mean completely different thing.
> > > > > How bonded device would know that to deal properly?
> > > > >
> > > > > Another example - user has 2 NICs of different type and would like to send the same packet on both of them simultaneously.
> > > > > As PKT_TX_RESERVED might mean different things for these devices, and user would like to use let say
> > > > > PKT_TX_IXGBE_MACSEC on one of them, he would need to do a copy of them, instead just increment a refcnt.
> > > > >
> > > > > Similar issues might arise at RX handling: user got a packet with PKT_RX_RESERVED_0 set.
> > > > > What does it really mean if there are different NIC types in the system?
> > > > > The only way to answer that question, as I can see,  is to keep track from what NIC that packet was received.
> > > > > Which I suppose, is not always convenient.
> > > > >
> > > >
> > > > The main purpose is to put the PMD-specific APIs in a separate
> > > > namespace instead of mixing the PMD-specific APIs and global APIs
> > > > up, and also save the bits in mbuf.ol_flags.
> > > >
> > > > There are other ways to achieve this goal, such as introducing
> > > > the PMD specific ol_flags in mbuf second cache line as you said.
> > > > I just thought defining some reserved bits seems to be the most
> > > > simple way which won't introduce many changes.
> > > >
> > > > What's your suggestions? Should I just revert the changes and
> > > > define the generic flags directly?
> > >
> > > Yes, that would be my preference.
> > > As I said above - spending extra bit in ol_flags  doesn't look like a big problem to me.
> > > In return there would be no need to consider how to handle all that confusing scenarios in future.
> >
> > Okay. I'll update my patches. Thanks a lot for your comments.
> 
> Well, I do not agree with Konstantin (no one saw this coming eh?)

:)

 >and do not think you need to update your series again.
> 
> PMD-specific symbols have nothing to do in the global namespace in my
> opinion, they are not versioned and may evolve without notice. Neither
> applications nor the bonding PMD can rely on them. That's the trade-off.

Not sure I do understand your reasoning.
For me MACSEC offload is just one of many HW offloads that we support
and should be treated in exactly the same way.
Applications should be able to use it in a transparent and reliable way,
not only under some limited conditions. 
Otherwise what is the point to introduce it at all?
Yes, right now it is supported only by ixgbe PMD, but why that should be the
reason to treat is as second-class citizen?
Let say PKT_TX_TUNNEL_* offloads also are supported only by one PMD right now.

> 
> Therefore until APIs are made global, the safe compromise is to define
> neutral, reserved symbols that any PMD can use to implement their own
> temporary APIs for testing purposes. These can be renamed later without
> changing their value as long as a single PMD uses them.

Ok, so what we'll gain by introducing PKT_TX_RESERVED instead of PKT_TX_MACSEC?
As I said in my previous mail the redefinition for the same ol_flag bit (and dev capabilities)
by different PMD might create a lot of confusion in future.
Does the potential saving of 1 bit really worth it?
 
Konstantin

  parent reply	other threads:[~2017-01-05 11:32 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-03 14:59 [PATCH 0/3] Add MACsec offload support for ixgbe Tiwei Bie
2016-12-03 14:59 ` [PATCH 1/3] lib: add MACsec offload flags Tiwei Bie
2016-12-03 14:59 ` [PATCH 2/3] net/ixgbe: add MACsec offload support Tiwei Bie
2016-12-03 14:59 ` [PATCH 3/3] app/testpmd: add ixgbe " Tiwei Bie
2016-12-16  1:43 ` [PATCH v2 0/4] Add MACsec offload support for ixgbe Tiwei Bie
2016-12-16  1:43   ` [PATCH v2 1/4] lib: add MACsec offload flags Tiwei Bie
2016-12-16  1:43   ` [PATCH v2 2/4] net/ixgbe: add MACsec offload support Tiwei Bie
2016-12-16  1:43   ` [PATCH v2 3/4] app/testpmd: add ixgbe " Tiwei Bie
2016-12-16  1:43   ` [PATCH v2 4/4] doc: add ixgbe specific APIs Tiwei Bie
2016-12-16 10:29     ` Mcnamara, John
2016-12-25 14:57   ` [PATCH v3 0/6] Add MACsec offload support for ixgbe Tiwei Bie
2016-12-25 14:57     ` [PATCH v3 1/6] mbuf: add flag for MACsec Tiwei Bie
2016-12-25 14:57     ` [PATCH v3 2/6] ethdev: add event type " Tiwei Bie
2016-12-25 14:57     ` [PATCH v3 3/6] ethdev: add MACsec offload capability flags Tiwei Bie
2016-12-25 14:57     ` [PATCH v3 4/6] net/ixgbe: add MACsec offload support Tiwei Bie
2016-12-25 14:57     ` [PATCH v3 5/6] app/testpmd: add MACsec offload commands Tiwei Bie
2016-12-25 14:58     ` [PATCH v3 6/6] doc: add ixgbe specific APIs Tiwei Bie
2016-12-26 15:15     ` [PATCH v3 0/6] Add MACsec offload support for ixgbe Adrien Mazarguil
2016-12-27  1:33       ` Tiwei Bie
2016-12-27 14:37         ` Adrien Mazarguil
2016-12-27 17:42           ` Tiwei Bie
2016-12-28 15:41     ` [PATCH v4 0/7] " Tiwei Bie
2016-12-28 15:41       ` [PATCH v4 1/7] mbuf: reserve a Tx offload flag for PMD-specific API Tiwei Bie
2016-12-28 15:41       ` [PATCH v4 2/7] ethdev: reserve an event type " Tiwei Bie
2016-12-28 15:41       ` [PATCH v4 3/7] ethdev: reserve capability flags " Tiwei Bie
2016-12-28 15:41       ` [PATCH v4 4/7] net/ixgbe: add MACsec offload support Tiwei Bie
2016-12-28 15:41       ` [PATCH v4 5/7] app/testpmd: add MACsec offload commands Tiwei Bie
2016-12-28 15:41       ` [PATCH v4 6/7] doc: add ixgbe specific APIs Tiwei Bie
2016-12-28 15:41       ` [PATCH v4 7/7] doc: update the release notes for the reserved flags Tiwei Bie
2017-01-03  6:15       ` [PATCH v4 0/7] Add MACsec offload support for ixgbe Lu, Wenzhuo
2017-01-04  7:21       ` [PATCH v5 0/8] " Tiwei Bie
2017-01-04  7:21         ` [PATCH v5 1/8] mbuf: reserve a Tx offload flag for PMD-specific API Tiwei Bie
2017-01-04  7:21         ` [PATCH v5 2/8] ethdev: reserve an event type " Tiwei Bie
2017-01-04  7:21         ` [PATCH v5 3/8] ethdev: reserve capability flags " Tiwei Bie
2017-01-04 14:21           ` Ananyev, Konstantin
2017-01-04 14:39             ` Tiwei Bie
2017-01-04 15:14               ` Ananyev, Konstantin
2017-01-04 17:00                 ` Tiwei Bie
2017-01-04 17:44                   ` Ananyev, Konstantin
2017-01-04 23:56                     ` Tiwei Bie
2017-01-05  8:33                       ` Adrien Mazarguil
2017-01-05 10:05                         ` Tiwei Bie
2017-01-05 11:32                         ` Ananyev, Konstantin [this message]
2017-01-05 18:21                           ` Adrien Mazarguil
2017-01-08 12:39                             ` Ananyev, Konstantin
2017-01-09  3:57                               ` Tiwei Bie
2017-01-09 11:26                                 ` Thomas Monjalon
2017-01-10  2:08                                   ` Tiwei Bie
2017-01-11 17:32                                   ` Olivier MATZ
2017-01-12  2:08                                     ` Tiwei Bie
2017-01-12  2:42                                     ` Yuanhan Liu
2017-01-09 14:41                               ` Adrien Mazarguil
2017-01-04  7:21         ` [PATCH v5 4/8] net/ixgbe: add MACsec offload support Tiwei Bie
2017-01-04  7:21         ` [PATCH v5 5/8] app/testpmd: add MACsec offload commands Tiwei Bie
2017-01-04  7:21         ` [PATCH v5 6/8] doc: add ixgbe specific APIs Tiwei Bie
2017-01-04  7:21         ` [PATCH v5 7/8] doc: update the release notes for the reserved flags Tiwei Bie
2017-01-04  7:21         ` [PATCH v5 8/8] doc: add MACsec offload into NIC feature list Tiwei Bie
2017-01-04  8:29         ` [PATCH v5 0/8] Add MACsec offload support for ixgbe Peng, Yuan
2017-01-11  4:31         ` [PATCH v6 0/6] " Tiwei Bie
2017-01-11  4:31           ` [PATCH v6 1/6] mbuf: add flag for MACsec Tiwei Bie
2017-01-11 17:41             ` Olivier MATZ
2017-01-11  4:31           ` [PATCH v6 2/6] ethdev: add event type " Tiwei Bie
2017-01-11  4:31           ` [PATCH v6 3/6] ethdev: add MACsec offload capability flags Tiwei Bie
2017-01-11  4:31           ` [PATCH v6 4/6] net/ixgbe: add MACsec offload support Tiwei Bie
2017-01-11  4:31           ` [PATCH v6 5/6] app/testpmd: add MACsec offload commands Tiwei Bie
2017-01-13  9:07             ` Thomas Monjalon
2017-01-13  9:16               ` Tiwei Bie
2017-01-11  4:31           ` [PATCH v6 6/6] doc: add ixgbe specific APIs Tiwei Bie
2017-01-13 11:21           ` [PATCH v7 0/6] Add MACsec offload support for ixgbe Tiwei Bie
2017-01-13 11:21             ` [PATCH v7 1/6] mbuf: add flag for MACsec Tiwei Bie
2017-01-13 11:21             ` [PATCH v7 2/6] ethdev: add event type " Tiwei Bie
2017-01-16  2:31               ` Ananyev, Konstantin
2017-01-13 11:21             ` [PATCH v7 3/6] ethdev: add MACsec offload capability flags Tiwei Bie
2017-01-16  2:33               ` Ananyev, Konstantin
2017-01-13 11:21             ` [PATCH v7 4/6] net/ixgbe: add MACsec offload support Tiwei Bie
2017-01-13 11:21             ` [PATCH v7 5/6] app/testpmd: add MACsec offload commands Tiwei Bie
2017-01-13 11:21             ` [PATCH v7 6/6] doc: add ixgbe specific APIs Tiwei Bie
2017-01-15 18:19             ` [PATCH v7 0/6] Add MACsec offload support for ixgbe Thomas Monjalon
2017-01-23  2:30             ` Peng, Yuan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2601191342CEEE43887BDE71AB9772583F1006B7@irsmsx105.ger.corp.intel.com \
    --to=konstantin.ananyev@intel.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=helin.zhang@intel.com \
    --cc=john.mcnamara@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=thomas.monjalon@6wind.com \
    --cc=tiwei.bie@intel.com \
    --cc=wei.dai@intel.com \
    --cc=wenzhuo.lu@intel.com \
    --cc=xiao.w.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.