DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	John McNamara <john.mcnamara@intel.com>,
	Marko Kovacevic <marko.kovacevic@intel.com>
Subject: Re: [dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function
Date: Fri, 01 Nov 2019 23:25:37 +0100	[thread overview]
Message-ID: <2639486.gPBSui7x2o@xps> (raw)
In-Reply-To: <55a8ec94-bfe8-d132-5122-d322f83f02b2@solarflare.com>

01/11/2019 11:55, Andrew Rybchenko:
> On 10/31/19 7:38 PM, Pavan Nikhilesh Bhagavatula wrote:
> >> 29/10/2019 16:37, pbhagavatula@marvell.com:
> >>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >>>  Removed Items
> >>>  -------------
> >>> --- a/lib/librte_ethdev/rte_ethdev.h
> >>> +++ b/lib/librte_ethdev/rte_ethdev.h
> >>> +/**
> >>> + * @warning
> >>> + * @b EXPERIMENTAL: this API may change without prior notice.
> >>> + *
> >>> + * Inform Ethernet device of the packet types classification the
> >> recipient is
> >>> + * interested in.
> >> This is a bit convoluted.
> >> What about this?
> >> "Optimize driver handling of packet types by reducing its range."
> > @arybchenko@solarflare.com Thoughts?
> 
> Optimize is a possible side effect of the operation, but there is
> no any promise that something will be optimized.
> I thought that current description explains what happens.
> Below statements try to explain why it may be useful.
> Any other options?

"Reduce range of packet types to handle."

> >>> + * Application can use this function to set only specific ptypes that it's
> >>> + * interested. This information can be used by the PMD to optimize
> >> Rx path.
> >>> + *
> >>> + * The function accepts an array `set_ptypes` allocated by the caller
> >> to
> >>> + * store the packet types set by the driver, the last element of the
> >> array
> >>> + * is set to RTE_PTYPE_UNKNOWN. The size of the `set_ptype` array
> >> should be
> >>> + * `rte_eth_dev_get_supported_ptypes() + 1` else it might only be
> >> filled
> >>> + * partially.
> >>> + *
> >>> + * @param port_id
> >>> + *   The port identifier of the Ethernet device.
> >>> + * @param ptype_mask
> >>> + *   The ptype family that application is interested in should be
> >> bitwise OR of
> >>> + *   RTE_PTYPE_*_MASK or 0.
> >>> + * @param set_ptypes
> >>> + *   An array pointer to store set packet types, allocated by caller. The
> >>> + *   function marks the end of array with RTE_PTYPE_UNKNOWN.
> >>> + * @param num
> >>> + *   Size of the array pointed by param ptypes.
> >>> + *   Should be rte_eth_dev_get_supported_ptypes() + 1 to
> >> accommodate the
> >>> + *   set ptypes.
> >>> + * @return
> >>> + *   - (0) if Success.
> >>> + *   - (-ENODEV) if *port_id* invalid.
> >>> + *   - (-EINVAL) if *ptype_mask* is invalid (or) set_ptypes is NULL and
> >>> + *     num > 0.
> >>> + */
> >> John, please you check the English wording?
> >>
> >>> +__rte_experimental
> >>> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
> >> ptype_mask,
> >>> +				     uint32_t *set_ptypes, unsigned int
> >> num);
> >>
> >> I don't like the name of the function because they are
> >> not "supported" packet types but "requested".
> >> What about replacing "set_supported" with "set_allowed", or
> >> "white_list"?
> > "white_list" seems ok but hope it doesn't call for blacklisting API.
> 
> "white_list" suggests that it is guaranteed that nothing else will
> be reported. At least for me. However, I agree that set_supported
> is not nice and I accepted it just to keep API naming symmetric.
> May be it is really misleading here. May be just: rte_eth_dev_set_ptypes()?

Maybe the word "allowed" would better fit?



  reply	other threads:[~2019-11-01 22:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-31 16:38 Pavan Nikhilesh Bhagavatula
2019-11-01 10:55 ` Andrew Rybchenko
2019-11-01 22:25   ` Thomas Monjalon [this message]
2019-11-05 12:42     ` Andrew Rybchenko
  -- strict thread matches above, loose matches on Subject: below --
2019-10-29  5:03 [dpdk-dev] [PATCH v14 0/6] ethdev: add new Rx offload flags pbhagavatula
2019-10-29 15:37 ` [dpdk-dev] [PATCH v15 0/7] " pbhagavatula
2019-10-29 15:37   ` [dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function pbhagavatula
2019-10-31 13:39     ` Thomas Monjalon

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=2639486.gPBSui7x2o@xps \
    --to=thomas@monjalon.net \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jerinj@marvell.com \
    --cc=john.mcnamara@intel.com \
    --cc=marko.kovacevic@intel.com \
    --cc=pbhagavatula@marvell.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).