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

[Ravi Kerur]

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.

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.

Let me know your inputs.

Thanks.

On Wed, Mar 1, 2017 at 7:29 AM, Ravi Kerur <rkerur at gmail.com> wrote:

> Hi Konstantin,
>
> Thank you for the review.
>
> RSS hash value changes could be due to merge, I didn't make that change. I
> will go through the changes and fix it in 'v2' patch along with RFC removed
> and checkpatch fix.
>
> Thanks.
>
> On Tue, Feb 28, 2017 at 2:36 AM, Ananyev, Konstantin <
> konstantin.ananyev at intel.com> wrote:
>
>> Hi Ravi,
>>
>> >
>> > Thanks to Konstantin and Bruce on first internal review comments. This
>> > patch is RFC for 17.05 to merge l3fwd-acl and l3fwd code and add file
>> > read options to build LPM and EM tables.
>>
>>
>> Thanks for the patch, I think it is really useful one.
>> Can I suggest you re-submit it as non-RFC now, as we are in 17.05 window
>> already?
>> About the patch itself, one question I forgot to ask you before:
>>
>> +++ 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,
>> +
>>                 },
>>         },
>>
>>
>> Why it is necessary to change RSS hash input values?
>>
>> As another nit - there are few checkpatch warnings, that probably need
>> to be addressed.
>> Apart from that looks good to me.
>> Thanks
>> Konstantin
>>
>>
>>
>