DPDK patches and discussions
 help / color / mirror / Atom feed
From: Vladimir Medvedkin <medvedkinv@gmail.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter
Date: Fri, 13 Jun 2014 15:51:31 +0400	[thread overview]
Message-ID: <CANDrEH=8SeES7N5eybWaCvJdE0JkZpq-DuxSVYdXAXbm=Wf-Ww@mail.gmail.com> (raw)
In-Reply-To: <27520622.tiYQNKLyvh@xps13>

Hi all,

The 82599 datasheet (p. 284 and p.287) has only recommendations and only
when possible about assign rx queue not used by RSS/DCB. I do not see any
serious restrictions do not assign the rx queue used by RSS/DCB.
For cases with only 1 queue if I understand correctly this patch
http://dpdk.org/ml/archives/dev/2014-May/002589.html we can init second
queue in pool and assign it by filter. In *ETH_MQ_RX_VMDQ_ONLY*  mode init
all possible queues (even if hardware route packets to zero queue in pools)
so there no problem. Moreover, it is not necesary for rx queue to be set in
the same pool.

About genericity. I agree with Jingjing, different controllers have
different definitions for pools or VFs. And it is only Intel controllers!
It is very hard to predict hardware implementation. For example for
Fortville I can not find 5-tuple filters at all.

API. I have several remarks.
1. You use rx_queue as separate arg. For example:
rte_eth_dev_add_ethertype_filter(uint8_t port_id, uint16_t index, struct
rte_ethertype_filter *filter, uint8_t rx_queue)
rte_eth_dev_get_ethertype_filter(uint8_t port_id, uint16_t index, struct
rte_ethertype_filter *filter, uint8_t *rx_queue)
you can move uint8_t rx_queue into struct rte_ethertype_filter *filter.
2. In SYN filter:
rte_eth_dev_add_syn_filter(uint8_t port_id, uint8_t high_pri, uint8_t
rx_queue)
rte_eth_dev_get_syn_filter(uint8_t port_id, struct rte_syn_filter *filter,
uint8_t *rx_queue)
In first ADD func you alloc struct rte_syn_filter inside func, but in GET
func you have to alloc struct rte_syn_filter in your app. May be better to
do
rte_eth_dev_add_syn_filter(uint8_t port_id, struct rte_syn_filter *filter,
uint8_t *rx_queue) ?

So, Jingjing made a lot of work, much more then I (igb filters, testpmd
commands). It works the same as mine (not counting pools logic), so let's
integrate it (it's will be great if jingjing change api according to my
remarks).

Regards,
Vladimir


2014-06-12 19:36 GMT+04:00 Thomas Monjalon <thomas.monjalon@6wind.com>:

> > 2014-06-11 17:45, Thomas Monjalon:
> > > My main concern is that Vladimir Medvedkin suggested another API and
> I'd
> > > like you give your opinion about it:
> > >             http://dpdk.org/ml/archives/dev/2014-June/003053.html
> > > It offers pool number in configuration of the filters.
>
> 2014-06-12 08:08, Wu, Jingjing:
> > The pool field is used in virtualization scenario. It is acting as one of
> > input set during filter matching in ixgbe.
> > My patch didn't consider the virtualization scenario in generic filter
> > feature. Because in 82599 datasheet, it is recommended to assign rx
> queues
> > not used by DCB/RSS, that is virtualization without RSS and DCB mode. For
> > this mode, current DPDK version makes the number of queue to 1 by
> default in
> > IOV mode. So in this case it makes no sense make pool as a input set and
> the
> > rx queue also need to be set to in this pool, so just keep the consistent
> > with flow director who also ignore it in previous version.
> > And further E1000/Niantic/Fortville have different definitions for VF, we
> > need to think how to define it more generic.
> > And even just need offer pool number in configuration of the filters as
> what
> > Vladimir did, it also need to verify the interworking with Virtualization
> > for different kinds of NICs, and the interworking with DCB and RSS which
> is
> > not recommended in 82599's datasheet.
> > So I think it will be a good choice to implement generic filter
> interworking
> > with virtualization in future patch. If there is any volunteer to send
> patch
> > for support this concern later, it will be also cool.
>
> Vladimir, do you agree with this analysis?
> As you suggested another implementation, I need you acknowledgment for this
> patchset to be integrated.
>
> Thanks
> --
> Thomas
>

  reply	other threads:[~2014-06-13 11:51 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-24  1:37 Jingjing Wu
2014-05-24  1:37 ` [dpdk-dev] [PATCH v2 1/4]ethdev: add ethdev APIs for NIC filters of " Jingjing Wu
2014-05-27  7:57   ` Ming, LiX
2014-05-27  9:18     ` Thomas Monjalon
2014-05-24  1:37 ` [dpdk-dev] [PATCH v2 2/4]e1000: add igb NIC filters of generic filter feature Jingjing Wu
2014-05-27  7:59   ` Ming, LiX
2014-05-24  1:37 ` [dpdk-dev] [PATCH v2 3/4]ixgbe: add ixgbe " Jingjing Wu
2014-05-27  8:00   ` Ming, LiX
2014-05-24  1:37 ` [dpdk-dev] [PATCH v2 4/4]app/test-pmd: add commands in testpmd for NIC filters Jingjing Wu
2014-05-27  8:00   ` Ming, LiX
2014-05-27 23:21 ` [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter Thomas Monjalon
2014-05-28  0:53   ` Wu, Jingjing
2014-05-28  1:12     ` Wu, Jingjing
2014-06-11 15:45       ` Thomas Monjalon
2014-06-12  8:08         ` Wu, Jingjing
2014-06-12 15:36           ` Thomas Monjalon
2014-06-13 11:51             ` Vladimir Medvedkin [this message]
2014-06-13 14:12               ` Wu, Jingjing
2014-06-13 16:19                 ` Vladimir Medvedkin
2014-06-14  1:00                   ` Wu, Jingjing
2014-06-14  9:00                     ` Vladimir Medvedkin
2014-06-04  4:16 ` Cao, Waterman

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='CANDrEH=8SeES7N5eybWaCvJdE0JkZpq-DuxSVYdXAXbm=Wf-Ww@mail.gmail.com' \
    --to=medvedkinv@gmail.com \
    --cc=dev@dpdk.org \
    --cc=thomas.monjalon@6wind.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).