On Sat, Oct 11, 2014 at 08:56:06PM -0400, Thomas F Herbert wrote:
> This is the linux kernel portion of the patch.
>
> Signed-off-by: Thomas F Herbert <thomasfherb...@entpnt.com>
In the subject, it's customary to put the version number inside the
brackets so that it doesn't get into the commit message.
Compiling against Linux 3.2 (I don't have any particular attachment to
that version, it's just what I'm using at the moment), I get a kernel
module build error:
/home/blp/ovs/_build/datapath/linux/flow.c: In function 'parse_vlan':
/home/blp/ovs/_build/datapath/linux/flow.c:314:6: error: 'struct sk_buff'
has no member named 'vlan_proto'
and these kernel module "sparse" warnings:
/home/blp/ovs/_build/datapath/linux/flow.c:315:44: warning: symbol 'qp'
shadows an earlier one
/home/blp/ovs/_build/datapath/linux/flow.c:301:28: originally declared here
/home/blp/ovs/_build/datapath/linux/flow.c:310:13: warning: restricted
__be16 degrades to integer
/home/blp/ovs/_build/datapath/linux/flow.c:312:48: warning: restricted
__be16 degrades to integer
/home/blp/ovs/_build/datapath/linux/flow.c:312:30: warning: incorrect type
in assignment (different base types)
/home/blp/ovs/_build/datapath/linux/flow.c:312:30: expected restricted
__be16 [usertype] tci
/home/blp/ovs/_build/datapath/linux/flow.c:312:30: got int
Also some userspace warnings:
../lib/odp-execute.c: In function 'odp_execute_set_action':
../lib/odp-execute.c:188:5: error: enumeration value 'OVS_KEY_ATTR_CVLAN'
not handled in switch [-Werror=switch-enum]
../lib/odp-execute.c: In function 'odp_execute_masked_set_action':
../lib/odp-execute.c:280:5: error: enumeration value 'OVS_KEY_ATTR_CVLAN'
not handled in switch [-Werror=switch-enum]
cc1: all warnings being treated as errors
make[3]: *** [lib/odp-execute.lo] Error 1
CC lib/odp-util.lo
../lib/odp-util.c: In function 'ovs_key_attr_to_string':
../lib/odp-util.c:103:5: error: enumeration value 'OVS_KEY_ATTR_CVLAN' not
handled in switch [-Werror=switch-enum]
../lib/odp-util.c: In function 'odp_flow_key_attr_len':
../lib/odp-util.c:938:5: error: enumeration value 'OVS_KEY_ATTR_CVLAN' not
handled in switch [-Werror=switch]
../lib/odp-util.c: In function 'format_odp_key_attr':
../lib/odp-util.c:1511:5: error: enumeration value 'OVS_KEY_ATTR_CVLAN' not
handled in switch [-Werror=switch-enum]
cc1: all warnings being treated as errors
In pop_vlan() in datapath/actions.c, the indentation here looks
randomly weird and I wonder whether you have a non-8-column tab stop:
+ if (unlikely(((skb->protocol != htons(ETH_P_8021Q)) &&
+ (skb->protocol != htons(ETH_P_8021AD))) ||
+ (skb->len < VLAN_ETH_HLEN)))
This patch adds a blank line at the beginning of push_vlan() that
looks like a mistake.
The ovs_key_attr values are part of the ABI so you shouldn't add a new
value in the middle.
Why is OVS_KEY_ATTR_CVLAN needed? The expectation when we designed
the Netlink flow structures was that nested VLANs would be implemented
as multiple nested attributes.
I didn't read all of the kernel code so this isn't a full review.
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
