[dpdk-dev] [RFC 17.05 v1 0/3] Merge l3fwd-acl and l3fwd

Ananyev, Konstantin konstantin.ananyev at intel.com
Sun Mar 5 14:15:15 CET 2017


Hi Ravi,

> 
> Hi Konstantin,
> 
> Sorry for this one, I had to resend patch series as 'v3' as additional checkpatch warnings were seen after the submission which didn't show
> up in my run.
> 
> 'v3' patch should have all fixed except the ones I mentioned in my earlier email on which I need inputs from you.
> 
> Thanks.
> 
> On Sat, Mar 4, 2017 at 11:49 AM, Ravi Kerur <rkerur at gmail.com> wrote:
> Hi Konstantin,
> 
> I have sent 'v2' patchset. I need clarifications on following things, if they should be fixed I will send out 'v3' so please let me know.
> 
> Following code changes were done by me manually, not merged.
> +++ b/examples/l3fwd/main.c
> @@ -161,7 +163,9 @@  static struct rte_eth_conf port_conf = {
>         .rx_adv_conf = {
>                 .rss_conf = {
>                         .rss_key = NULL,
> -                       .rss_hf = ETH_RSS_IP,
> +                       .rss_hf = ETH_RSS_IP | ETH_RSS_UDP |
> +                               ETH_RSS_TCP | ETH_RSS_SCTP,
> +
>                 },
> 
> The reason I did it is because
> 
> LPM/EM has .rss_hf = ETH_RSS_IP
> ACL has .rss_hf = ETH_RSS_IP | ETH_RSS_UDP | ETH_RSS_TCP | ETH_RSS_SCTP,
> 
> ACL looks like a superset of LPM/EM and functional testing didn't reveal any issues hence I kept ACL version.

But at least for LPM, we probably don't want L4 ports affect packet distribution?
Probably the safest way would be to have a separate port_conf for each case (LPM/EM/ACL).
That way will preserve the original behavior.
 
> 
> 2. Checkpatch errors are all fixed. Some warnings are not fixed and they are
> 
> 2.a, string length greater than 80 characters
> 2.b GET_CB_FIELD macro. I could have changed GET_CB_FIELD to inline function, however, function names cannot be in capital letters. I
> could have changed it to 'get_cb_field' inline function, but didn't do it as I thought it may not be worth the change.

It is ok by me to leave as it is by now.
Thanks
Konstantin





More information about the dev mailing list