On Fri, May 12, 2017 at 11:07:46AM +0000, Zoltán Balogh wrote:
> From: Georg Schmuecking <[email protected]>
>
> This patch is based on the "datapath: enable vxlangpe creation in compat mode"
> from Yi Yang. It introduces an extension option "gpe" to the vxlan port in the
> netdev-dpdk datapath. Furthermore it introduces a new protocol header file
> vxlangpe.h in which the vxlan gpe protocoll is described. In the vxlan
> specific
> methods the different packet are introduced and handled.
>
> Added VXLAN GPE tunnel push test.
>
> Signed-off-by: Yi Yang <yi.y.yang at intel.com>
> Signed-off-by: Georg Schmuecking <[email protected]>
checkpatch says:
ERROR: Inappropriate bracing around statement
#138 FILE: lib/netdev-native-tnl.c:519:
if (gpe->oam_flag)
WARNING: Line has non-spaces leading whitespace
#153 FILE: lib/netdev-native-tnl.c:534:
}
WARNING: Line length is >79-characters long
#188 FILE: lib/netdev-native-tnl.c:578:
put_16aligned_be32(&vxh->vx_vni,
htonl(ntohll(params->flow->tunnel.tun_id) << 8));
WARNING: Line length is >79-characters long
#206 FILE: lib/netdev-native-tnl.c:596:
put_16aligned_be32(&vxh->vx_vni,
htonl(ntohll(params->flow->tunnel.tun_id) << 8));
In netdev_vxlan_pop_header(), I don't see anything that verifies that
the packet is long enough when GPE is present.
The formatting of the switch statements look different from the usual
OVS style, which is like this:
switch (gpe->next_protocol) {
case VXLAN_GPE_NP_IPV4:
next_pt = PT_IPV4;
break;
case VXLAN_GPE_NP_IPV6:
next_pt = PT_IPV6;
break;
case VXLAN_GPE_NP_ETHERNET:
next_pt = PT_ETH;
break;
default:
goto err;
}
I suspect that struct vxlanhdr_gpe should use ovs_16aligned_be32 for the
vx_vni member.
I doubt that these #defines are a good idea, in vxlangpe.h:
#define u8 uint8_t
#define u32 uint8_t
#define __be32 ovs_be32
struct vxlanhdr in lib/packets.h is pretty different in philosophy from
struct vxlanhdr_gpe in vxlangpe.h. I'd suggest making the new structure
more like the existing one; OVS doesn't really use bit-fields much and
in my experience they do not make code much easier to deal with.
The new struct vxlan_metadata doesn't seem to be used anywhere and I
wonder whether it should be included.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
