Greg, thank you so much for your reviewing, which Linux distribution version 
has linux-3.10.107 kernel?  I’ll add a test case for kernel datapath, fix 
compiling issue on linux-3.10.107 and rebase it to master.

From: Gregory Rose [mailto:gvrose8...@gmail.com]
Sent: Tuesday, January 30, 2018 1:51 AM
To: Yang, Yi Y <yi.y.y...@intel.com>; d...@openvswitch.org
Cc: b...@ovn.org; jan.scheur...@ericsson.com
Subject: Re: [PATCH v1 0/5] datapath: enable NSH support in kernel compat mode

On 1/10/2018 11:53 PM, Yi Yang wrote:


This patch series is to backport NSH support patches in Linux net-next tree

to OVS in order that it can support NSH in kernel compat mode.



Yi Yang (5):

  datapath: ether: add NSH ethertype

  datapath: vxlan: factor out VXLAN-GPE next protocol

  datapath: net: add NSH header structures and helpers

  datapath: nsh: add GSO support

  datapath: enable NSH support



 NEWS                                              |   1 +

 datapath/Modules.mk                               |   4 +-

 datapath/actions.c                                | 116 ++++++++

 datapath/datapath.c                               |   4 +

 datapath/flow.c                                   |  51 ++++

 datapath/flow.h                                   |   7 +

 datapath/flow_netlink.c                           | 343 +++++++++++++++++++++-

 datapath/flow_netlink.h                           |   5 +

 datapath/linux/Modules.mk                         |   2 +

 datapath/linux/compat/include/linux/if_ether.h    |   4 +

 datapath/linux/compat/include/linux/openvswitch.h |   6 +-

 datapath/linux/compat/include/net/nsh.h           | 313 ++++++++++++++++++++

 datapath/linux/compat/include/net/tun_proto.h     |  49 ++++

 datapath/linux/compat/include/net/vxlan.h         |   6 -

 datapath/linux/compat/vxlan.c                     |  32 +-

 datapath/nsh.c                                    | 142 +++++++++

 16 files changed, 1048 insertions(+), 37 deletions(-)

 create mode 100644 datapath/linux/compat/include/net/nsh.h

 create mode 100644 datapath/linux/compat/include/net/tun_proto.h

 create mode 100644 datapath/nsh.c



Hi Yi,

My apologies for the delay in reviewing this series.

I've finished up my review and I think it mostly looks pretty good but I did 
find an issue compiling on a 3.10.107 kernel build:
CC [M] 
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/vport-netdev.o
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/nsh.c:108:17: 
error: undefined identifier 'skb_gso_error_unwind'
CC [M] /home/travis/build/gvrose8192/ovs-experimental/datapath/linux/nsh.o
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/nsh.c: In 
function ‘nsh_gso_segment’:
/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/nsh.c:108:3: 
error: implicit declaration of function ‘skb_gso_error_unwind’ 
[-Werror=implicit-function-declaration]
skb_gso_error_unwind(skb, htons(ETH_P_NSH), nsh_len,
^
cc1: some warnings being treated as errors
make[3]: *** 
[/home/travis/build/gvrose8192/ovs-experimental/datapath/linux/nsh.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** 
[_module_/home/travis/build/gvrose8192/ovs-experimental/datapath/linux] Error 2
make[2]: Leaving directory 
`/home/travis/build/gvrose8192/ovs-experimental/linux-3.10.107'
make[1]: *** [default] Error 2
make[1]: Leaving directory 
`/home/travis/build/gvrose8192/ovs-experimental/datapath/linux'
make: *** [all-recursive] Error 1

So we'll need to fix that up and I also think the patches will need to be 
rebased to current master.  That second part is my fault... so sorry again 
about that.

One other thing, I ran this through our standard 'make check and make 
check-kmod' tests and everything was fine so the patches don't seem break 
anything.  I'm still concerned though that the test coverage probably didn't 
hit any parts of your code.  I'm wondering if there is some way I can test the 
code path and get some test coverage there.  Could you write up a self test for 
the tests/system-traffic.at kernel test?  Of if that's not practical is there 
some other way I could test this code?

Thanks,

- Greg

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to