DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ori Kam <orika@nvidia.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>,
	Gregory Etelson <getelson@nvidia.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Beilei Xing <beilei.xing@intel.com>,
	Bernard Iremonger <bernard.iremonger@intel.com>,
	Thomas Monjalon <tmonjalon@nvidia.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: Matan Azrad <matan@nvidia.com>,
	Raslan Darawsheh <rasland@nvidia.com>,
	Jeff Guo <jia.guo@intel.com>, Qi Zhang <qi.z.zhang@intel.com>
Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix flow rules list after port stop
Date: Sun, 13 Sep 2020 12:12:08 +0000
Message-ID: <MN2PR12MB42864043891EBCE913333480D6220@MN2PR12MB4286.namprd12.prod.outlook.com> (raw)
In-Reply-To: <MN2PR12MB428653C570339E13EEC9E9BBD6540@MN2PR12MB4286.namprd12.prod.outlook.com>

Hi Ferruh,
Can we proceed with this patch?

Thanks,
Ori
> -----Original Message-----
> From: Ori Kam
> 
> Hi Ferruh,
> 
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> >
> > On 8/20/2020 9:40 AM, Gregory Etelson wrote:
> > > Hello,
> > >
> > > Is this patch scheduled for merge with dpdk.org ?
> > > Please update me.
> > >
> > > Regards,
> > > Gregory
> > >
> > >> -----Original Message-----
> > >> From: Gregory Etelson <getelson@mellanox.com>
> > >>
> > >> According to current RTE API, port flow rules must not be kept after port
> > >> stop.
> >
> > Hi Gregory, Ori,
> >
> > Can you please point where this is documented?
> >
> From: rte_ethdev.h
> "Please note that some configuration is not stored between calls to
>  rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration will
>  be retained:
> 
>      - MTU
>      - flow control settings
>      - receive mode configuration (promiscuous mode, all-multicast mode,
>        hardware checksum mode, RSS/VMDQ settings etc.)
>      - VLAN filtering configuration
>      - default MAC address
>      - MAC addresses supplied to MAC address array
>      - flow director filtering mode (but not filtering rules)
>      - NIC queue statistics mappings"
> 
> From my understanding this means that flows should not be stored on device
> stop.
> 
> 
> > >>
> > >> Testpmd did not flush port flow rules after `port stop' command was
> called.
> > >> As the result, after the port was restarted, it showed bogus flow rules.
> >
> > There are two issues,
> >
> > 1) According what I see in the rte_flow documentation, not sure if the "port
> > stop" should clear the rules:
> > "
> > PMDs, not applications, are responsible for maintaining flow rules
> > configuration
> > when stopping and restarting a port or performing other actions which may
> > affect
> > them. They can only be destroyed explicitly by applications.
> > "
> >
> Good catch I think this part should be removed, since it has many issues. The
> application is the only
> one that can be responsible for the rules.
> 
> Thinks about the following scenario: application configures 2 queues 0 and 1.
> It insert flow with queue action 1.
> It stops the port and remove queue 1. What should the PMD do?
> What happens if he changed some thing else in configuration that make
> the actions invalid?
> 
> For those reason (the description in rte_ethdev.h and the above issues with
> keeping the rules)
> we (Mellanox) modified our code to remove the flows in stop function from the
> device.
> This code was inserted to DPDK in 20.05 release.
> One more reason is that saving the flows also waste a lot of memory
> which is very costly to many applications.
> 
> 
> > As I tested with i40e, it keeps the rules after stop/start, cc'ing @Jeff,
> > @Beilei & @Qi if this is done intentionally.
> >
> >
> > 2) From the perspective of the testers, users of the testpmd. If they are
> > testing a complex set of filter rules, stopping and starting the port flushing
> > all rules may be troublesome.
> > Since there is explicit command to remove a rte_flow rule or to remove them
> > all,
> > user may prefer to call it when required to delete the rules, instead of this is
> > done implicitly in port stop.
> >
> > Btw, this is based on PMD should handle the rules on stop/start, we need to
> > agree on it first, but even that is not the case, we are in the application
> > domain now and we can apply the rules back again in the 'start' if it serves
> > better to the user.
> >
> First like I said above I think we should agree that it is the application
> responsibility to manage the rules and not the PMD, and first thing to do it
> update the rte_flow doc.
> 
> Second I agree that we should discuss if test-pmd should keep the rules and
> reapply them,
> but just like for the PMD the user may create invalid configuration, so re-
> applying the rules
> maybe incorrect.
> Currently test-pmd is not build to support large number of rules, unless using a
> script, and if the user uses a script
> he can reuse this script.
> 
> 
> 
> Best,
> Ori


  reply	other threads:[~2020-09-13 12:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200810161523.6904-1-getelson@mellanox.com>
2020-08-20  8:40 ` Gregory Etelson
2020-08-25 15:51   ` Ferruh Yigit
2020-08-26 17:34     ` Ori Kam
2020-09-13 12:12       ` Ori Kam [this message]
2020-09-13 14:00         ` Thomas Monjalon
2020-11-17 19:08           ` Gregory Etelson
2020-08-11  6:14 Gregory Etelson
2020-11-24 14:42 ` Ajit Khaparde
2020-11-26 15:41   ` 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=MN2PR12MB42864043891EBCE913333480D6220@MN2PR12MB4286.namprd12.prod.outlook.com \
    --to=orika@nvidia.com \
    --cc=beilei.xing@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=getelson@nvidia.com \
    --cc=jia.guo@intel.com \
    --cc=matan@nvidia.com \
    --cc=qi.z.zhang@intel.com \
    --cc=rasland@nvidia.com \
    --cc=tmonjalon@nvidia.com \
    --cc=wenzhuo.lu@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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git