From: Tom Barbette <firstname.lastname@example.org> To: Andrew Rybchenko <email@example.com>, Ori Kam <firstname.lastname@example.org>, Thomas Monjalon <email@example.com> Cc: "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, "John McNamara" <firstname.lastname@example.org>, Marko Kovacevic <email@example.com>, Adrien Mazarguil <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, Olivier Matz <email@example.com> Subject: Re: [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update as an offload Date: Fri, 8 Nov 2019 09:00:33 +0000 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> My two cents as a regular user of the flow and mark feature: (A) seems much better and natural. In packet processing platform such as FastClick, I don't have a real understanding of the rules of the user, and rules can be added by the user while the application is running via a socket or web interface. So it is much easier to "pass the requirement", asking the user to give e.g. "FLOW_MARK true" to the I/O elements to enable the feature for later use that I easily translate to the offload flag. Moreover, that creates a dependency issue as I first initialize I/O elements so other elements can query some resources such as the MAC address (One could have half-initialization or a real dependency scheme but that is not supported for now). In the dynamic case (B) would require to forge a fake rule in case the feature is later used. (C) seems even more complex... Though if you want to use a dynamic field for the mark/meta, it seems okay to have a second requirement to add a field for it. Tom ________________________________________ De : dev <firstname.lastname@example.org> de la part de Andrew Rybchenko <email@example.com> Envoyé : vendredi 8 novembre 2019 09:35 À : Ori Kam; Thomas Monjalon Cc : firstname.lastname@example.org; email@example.com; firstname.lastname@example.org; email@example.com; John McNamara; Marko Kovacevic; Adrien Mazarguil; firstname.lastname@example.org; email@example.com; Olivier Matz Objet : Re: [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update as an offload On 11/6/19 10:42 AM, Ori Kam wrote: > >> -----Original Message----- >> From: Andrew Rybchenko <firstname.lastname@example.org> >> Sent: Wednesday, November 6, 2019 8:41 AM >> To: Ori Kam <email@example.com>; Thomas Monjalon >> <firstname.lastname@example.org> >> Cc: email@example.com; firstname.lastname@example.org; email@example.com; >> firstname.lastname@example.org; John McNamara <email@example.com>; Marko >> Kovacevic <firstname.lastname@example.org>; Adrien Mazarguil >> <email@example.com>; firstname.lastname@example.org; >> email@example.com >> Subject: Re: [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update as an >> offload >> >> On 11/5/19 7:37 PM, Ori Kam wrote: >>> >>>> -----Original Message----- >>>> From: Andrew Rybchenko <firstname.lastname@example.org> >>>> Sent: Tuesday, November 5, 2019 1:31 PM >>>> To: Ori Kam <email@example.com>; Thomas Monjalon >>>> <firstname.lastname@example.org> >>>> Cc: email@example.com; firstname.lastname@example.org; email@example.com; >>>> firstname.lastname@example.org; John McNamara <email@example.com>; Marko >>>> Kovacevic <firstname.lastname@example.org>; Adrien Mazarguil >>>> <email@example.com>; firstname.lastname@example.org; >>>> email@example.com >>>> Subject: Re: [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update as an >>>> offload >>>> >>> >>> [Snip] >>> >>>>> >>>>> Yes but like I said in Mellanox PMD for example we supported the mark only on non-transfer flows until this release. >>>>> so when the user set mark on transfer flow it was invalid. (in transfer flow if we have a miss we send the packet back to the Rx >>>>> port so the application can understand on which table the miss happened) >>>>> In this version we added the support for mark also in transfer (E-Switch) flows. >>>>> So my question before this release what should the PMD report? What should the PMD report after this release? >>>>> >>>>> Your idea was our first thought when adding the Tx meta, in that case the meta was always set in application >>>>> so we thought that this offload will enable us better function selection, but as you know we removed this capability >>>>> since it is not correct any more. >>>>> >>>>>> The above also highlights problems of the meta vs mark design. They are very >>>>>> similar and there is no any good definition of the difference and rules >>>>>> which one should be used/supported in which conditions. >>>>>> >>>>> >>>>> Mark and Meta are exactly the same, the meta is just another value that the application can use. >>>>> This is why both should act the same. >>>>> >>>>> And maybe this is the wining argument, the rte_flow validation approach was used and accepted for the meta. >>>>> So lets try it also with the mark. (please also remember that we didn't have this mark until now to somehow the >>>>> PMD worked 😊) >>>>> >>>>> Like I said before, I understand your approach, and each one of them has its own advantages and draw backs. >>>>> Lets start using the rte_flow approach and see how it goes, I promise you that if I see that it doesn't scale or cause more >>>>> issues I will be first one to submit changes. >>>> >>>> I tend to say OK, let's try. However, it must be documented >>>> in MARK action that if an application wants to use it, a rule >>>> with the action must be validated before device start. >>> >>> I agree to add this to the rte_flow mark action documation. >>> >>>> PMD may use the attempt as an indication from the application >>>> that it would like to use flow mark even if the validation >>>> fails. >>> >>> No if the PMD uses this validation as hint it should return success and >>> use the correct PMD. >> >> It would make it too strictly dependent on pattern/actions/state. > > I'm not sure I understand your comment. > Why would it make it to strict? I guess that mark action is doesn't care which items are in > the flow, so just setting eth item sound good enough. The problem is if single pattern+actions is used to check MARK support and let PMD know that application is going to use it and the flow rule is not supported, but another combination is supported, that's it. The feature is false classified as not supported. > Also I guess the mark doesn't conflict with many actions maybe the meta. > So the application doesn't have a lot of choices. > In any case and this is the most important one. In any case the application must validate that the > Nic support the mark in any case, according to RTE_FLOW definition. What is the meaning > of saying we support offload (using your segguetion) and then each flow you try to insert > will fail. So in any case the application must validate the flows it is going to use. Yes, but the decision here is per rule, not global. >>>> Ori, please, suggest formalized pattern and actions >>>> specification to use if application wants to utilize >>>> validation result as a criteria to enable/disable flow >>>> marks usage. >>> >>> I can’t do that, it depends on the application, the most basic is just "pattern eth actions mark / queue" . >>> In some cases where you need it for E-Switch if should be something like"transfer items port / eth / actions mark" >> >> If so, what application author should do if even maintainers cannot >> formalize it. It sounds like the approach does not work. > > This should be very easy for the application to know. How can I a mantianer know > what flows and what is important to the application, I need to make sure that the application > can understand if what it wants is supported. Yes and it is the problem of complex logic to make global decision at start of day. Flow API allows to avoid global decisions and verdict is returned for each specific rule, but here we need global decision which will affect further logic. >>>> What should be done if patterns to use and set >>>> of actions together with MARK are not known in advance. >>> >>> I think that the application knows which kind of traffic it expects and which actions it needs. >> >> I'm afraid it is not always true. > > Can you please give me such example, how can you develop an application without knowing what it should do? > This means the application is not optimized and impossible to test. First of all testpmd. I know that it is a test application etc, but I think it is still valid example. Second OVS, if I understand it correctly. As far as I know it allows to add a rule with various patterns and, in theory, it could result in the pattern to be used with MARK action. Basically any application which allows to add custom flow rules. I think we are running in circles here. I'll try to summarize the result of the discussion to make it easier for other developers to follow. Please, correct me if I'm wrong below. Sorry for a very-very long mail below, but I'd really like to finalize the discussion. The problem: ~~~~~~~~~~~~ PMD wants to know before port start if application wants to to use flow MARK/FLAG in the future. It is required because: 1. HW may be configured in a different way to reserve resources for MARK/FLAG delivery 2. Datapath implementation choice may depend on it (e.g. vPMD is faster, but does not support MARK) Discussed solutions: ~~~~~~~~~~~~~~~~~~~~ A. Explicit Rx offload suggested by the patch. B. Implicit by validation of a flow rule with MARK/FLAG actions used. C. Use dynamic field/flag (i.e. application registers dynamic field and/or flag and PMD uses lookup to solve the problem) plus part of (B) to discover if a feature is supported. All solutions require changes in applications which use these features. There is a deprecation notice in place which advertises DEV_RX_OFFLOAD_FLOW_MARK addition, but may be it is OK to substitute it with solution (B) or (C). Solution (C) requires changes since it should be combined with (B) in order to understand if the feature is supported. Advantages and drawbacks of solutions: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 1. The main drawback of (A) is a "duplication" since we already have a way to request flow MARK using rte_flow API. I don't fully agree that it is a duplication, but I agree that it sounds like duplication and complicates a bit flow MARK usage by applications. (B) complicates it as well. 2. One more drawback of the solution (A) is the necessity of similar solution for META and it eats one more offload bit. Yes, that's true and I think it is not a problem. It would make it easier for applications to find out if either MARK or META is supported. 3. The main advantage of the solution (A) is simplicity. It is simple for application to understand if it supported. It is simple in PMD to understand that it is required. It is simple to disable it - just reconfigure. Also it is easier to document it - just mention that the offload should be supported and enabled. 4. The main advantage of the solution (B) is no "duplication". I agree that it is valid argument. Solving the problem without extra entities is always nice, but unfortunately it is too complex in this case. 5. The main drawback of the solution (B) is the complexity. It is necessary to choose a flow rule which should be used as a criteria. It could be hardware dependent. Complex logic is require in PMD if it wants to address the problem and control MARK delivery based on validated flow rules. It adds dependency between start/stop processing and flow rules validation code. It is pretty complicated to document it. 6. Useless enabling of the offload in the case of solution (A) if really used flow rules do not support MARK looks like drawback as well, but easily mitigated by a combination with solution (B) and only required if the application wants to dive in the level of optimization and complexity and makes sense if application knows required flow rules in advance. So, it is not a problem in this case. 7. Solution (C) has drawbacks of the solution (B) for applications to understand if these features are supported, but no drawbacks in PMD, since explicit criteria is used to enable/disable (dynamic field/flag lookup). 8. Solution (C) is nice since it avoids "duplication". 9. The main drawback of the solution (C) is asymmetry. As it was discussed in the case of RX_TIMESTAMP (if I remember it correctly): - PMD advertises RX_TIMESTAMP offload capability - application enables the offload - PMD registers dynamic field for timestamp Solution (C): - PMD advertises nothing - application uses solution (B) to understand if these features are supported - application registers dynamic field/flag - PMD does lookup and solve the problem The asymmetry could be partially mitigated if RX_TIMESTAMP solution is changed to require an application to register dynamic fields and PMD to do lookup if the offload is enabled. So, the only difference will be in no offload in the case of flow MARK/FLAG and usage of complex logic to understand if it is supported or no. May be it would be really good since it will allow to have dynamic fields registered before mempool population. 10. Common drawback of solutions (B) and (C) is no granularity. Solution (A) may be per queue while (B) and (C) cannot be per queue. Moreover (C) looks global - for all devices. It could be really painful. (C) is nice, but I still vote for simplicity and granularity of (A). Andrew.
next prev parent reply index Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-25 15:21 pbhagavatula 2019-10-25 15:21 ` [dpdk-dev] [PATCH 2/2] drivers/net: update Rx flow flag and mark capabilities pbhagavatula 2019-10-28 10:50 ` [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update as an offload Ori Kam 2019-10-28 11:53 ` Andrew Rybchenko 2019-10-28 14:00 ` Ori Kam 2019-10-31 9:49 ` Andrew Rybchenko 2019-10-31 14:49 ` Thomas Monjalon 2019-10-31 23:59 ` Zhang, Qi Z 2019-11-01 11:35 ` Andrew Rybchenko 2019-11-03 10:22 ` Ori Kam 2019-11-03 11:41 ` Andrew Rybchenko 2019-11-04 18:37 ` Ori Kam 2019-11-05 6:50 ` Andrew Rybchenko 2019-11-05 8:35 ` Ori Kam 2019-11-05 11:30 ` Andrew Rybchenko 2019-11-05 16:37 ` Ori Kam 2019-11-06 6:40 ` Andrew Rybchenko 2019-11-06 7:42 ` Ori Kam 2019-11-08 8:35 ` Andrew Rybchenko 2019-11-08 9:00 ` Tom Barbette [this message] 2019-11-08 10:28 ` Thomas Monjalon 2019-11-08 10:42 ` Andrew Rybchenko 2019-11-08 11:03 ` Thomas Monjalon 2019-11-08 11:40 ` Zhang, Qi Z 2019-11-08 12:12 ` Ori Kam 2019-11-08 12:20 ` Andrew Rybchenko 2019-11-08 12:42 ` Ori Kam 2019-11-08 13:16 ` Zhang, Qi Z 2019-11-08 13:26 ` Thomas Monjalon 2019-11-08 13:06 ` Thomas Monjalon 2019-11-08 12:00 ` Andrew Rybchenko 2019-11-08 13:17 ` Thomas Monjalon 2019-11-08 13:27 ` Andrew Rybchenko 2019-11-08 13:30 ` Thomas Monjalon 2019-11-19 9:24 ` Andrew Rybchenko 2019-11-19 9:50 ` Thomas Monjalon 2019-11-19 10:59 ` Andrew Rybchenko 2019-11-19 11:09 ` Thomas Monjalon
Reply instructions: You may reply publically 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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 Archives are clonable: git clone --mirror http://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/ http://inbox.dpdk.org/dev \ email@example.com public-inbox-index dev Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/ public-inbox