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

[Ravi Kerur]

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.
>
> 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
>>>
>>>
>>>
>>
>