On 10/7/2020 4:43 PM, Ophir Munk wrote:
Hi Ferruh,
I sent V6 which addresses your last review.

Please see more comments inline.

-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@intel.com>
Sent: Tuesday, October 6, 2020 5:59 PM
On 9/18/2020 3:17 PM, Ophir Munk wrote:
v1:
Initial version
v2:
Rebased + Minor update in protocol options field:
char opts[0] ===> uint8_t opts[]
v3:
Rebase
document "geneve-port=N" parameter
v4:
Mispelling corrections
v5:
Rebase + Updates following review

https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch

es.dpdk.org%2Fpatch%2F77734%2F&amp;data=02%7C01%7Cophirmu%40nv
idia.com

%7C1b01b5de39d24d61e38008d86a086ae2%7C43083d15727340c1b7db39e
fd9ccc17a

%7C0%7C0%7C637375931691373733&amp;sdata=csLb5OdTmWlpv1k4Z7ZZ
YN1b1d2cd8
%2BTxxnydgNnyQ4%3D&amp;reserved=0

Ophir Munk (3):
    app/testpmd: add GENEVE parsing
    app/testpmd: enable configuring GENEVE port
    app/testpmd: reduce tunnel parsing code duplication


Hi Ophir,

The patchset looks good except a few comments I put into the patches.

But I have two highlevel questions/comments,

1) The testpmd tunnel parsing feature is not documented properly, there are
various related commands but there is no documentation to put all together.
What do you think putting a new section for it under the "Testpmd Runtime
Functions" (testpmd_funcs.rst) with this patchset?

I prefer this to be in a separate patchset.
Geneve in testpmd is not complete yet. I know that there is current work in 
progress to add geneve options to flows.
Maybe its worth waiting till Geneve is finalized in testpmd.


2) The 'csum' forwarding engine seems become forwarding engine for the
case where packet payload needs to be parsed, like gro/gso, tunnel parse.
Even the description of the forwarding engine in the documentation is not
accurate now.
I wonder if we should rename the forwarding engine at this stage?

Can you please be more specific to which renaming you are referring to?
Can you give examples?


I am asking about forwarding engine name, it is 'csum' and in documentation it is described as:
"
Changes the checksum field with hardware or software methods depending on the offload flags on the packet.
"

So the initial purpose of the forwarding engine is to calculate checksums (or offload calculation to HW) before sending packets, but now it detects and parses the tunneling protocols and updates packet accordingly and logs it.
It also does GRO and GSO.

Is the forwarding engine name 'csum' still make sense, or should we find something else. I don't have any suggestion, just the question.

Reply via email to