Message ID | 20170718223244.28449-2-joe@ovn.org |
---|---|
State | Accepted |
Headers | show |
On Tue, Jul 18, 2017 at 03:32:44PM -0700, Joe Stringer wrote: > For non-Ethernet flows, when fixing up the netlink message we need make > sure to pass down a valid Ethertype. The kernel does not understand > packet_type so it's implicitly encoded by the absence of _ETHERNET and > exact match of _ETHERTYPE. Without this change match_validate in the > kernel complains when trying to match packets from L3 tunnels. > e.g. > openvswitch: netlink: Unexpected mask (mask=110088, allowed=3d9804c) > > The mask use to always be set in xlate_wc_init() and xlate_wc_finish(), > but that changed for non-Ethernet frames with the commit listed below. > > Fixes: 3d4b2e6eb74e ("userspace: Add OXM field MFF_PACKET_TYPE") > Signed-off-by: Joe Stringer <joe@ovn.org> > Co-authored-by: Eric Garver <e@erig.me> > --- > lib/dpif-netlink.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index 63ef3de5c7d1..55effd1f91a8 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -3434,8 +3434,9 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow *flow, > > > /* > - * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out, > - * then puts 'data' to 'buf'. > + * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out. > + * If the flow is not Ethernet, the OVS_KEY_ATTR_PACKET_TYPE is converted to > + * OVS_KEY_ATTR_ETHERTYPE. Puts 'data' to 'buf'. > */ > static void > put_exclude_packet_type(struct ofpbuf *buf, uint16_t type, > @@ -3458,6 +3459,20 @@ put_exclude_packet_type(struct ofpbuf *buf, uint16_t type, > ofs = nl_msg_start_nested(buf, type); > nl_msg_put(buf, data, first_chunk_size); > nl_msg_put(buf, next_attr, second_chunk_size); > + if (!nl_attr_find__(data, data_len, OVS_KEY_ATTR_ETHERNET)) { > + ovs_be16 pt = pt_ns_type_be(nl_attr_get_be32(packet_type)); > + const struct nlattr *nla; > + > + nla = nl_attr_find(buf, NLA_HDRLEN, OVS_KEY_ATTR_ETHERTYPE); > + if (nla) { > + ovs_be16 *ethertype; > + > + ethertype = CONST_CAST(ovs_be16 *, nl_attr_get(nla)); > + *ethertype = pt; > + } else { > + nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, pt); > + } > + } > nl_msg_end_nested(buf, ofs); > } else { > nl_msg_put_unspec(buf, type, data, data_len); > -- > 2.11.1 > Thanks Joe! This is definitely more concise than my version. Acked-by: Eric Garver <e@erig.me>
On 19 July 2017 at 08:56, Eric Garver <e@erig.me> wrote: > On Tue, Jul 18, 2017 at 03:32:44PM -0700, Joe Stringer wrote: >> For non-Ethernet flows, when fixing up the netlink message we need make >> sure to pass down a valid Ethertype. The kernel does not understand >> packet_type so it's implicitly encoded by the absence of _ETHERNET and >> exact match of _ETHERTYPE. Without this change match_validate in the >> kernel complains when trying to match packets from L3 tunnels. >> e.g. >> openvswitch: netlink: Unexpected mask (mask=110088, allowed=3d9804c) >> >> The mask use to always be set in xlate_wc_init() and xlate_wc_finish(), >> but that changed for non-Ethernet frames with the commit listed below. >> >> Fixes: 3d4b2e6eb74e ("userspace: Add OXM field MFF_PACKET_TYPE") >> Signed-off-by: Joe Stringer <joe@ovn.org> >> Co-authored-by: Eric Garver <e@erig.me> >> --- >> lib/dpif-netlink.c | 19 +++++++++++++++++-- >> 1 file changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c >> index 63ef3de5c7d1..55effd1f91a8 100644 >> --- a/lib/dpif-netlink.c >> +++ b/lib/dpif-netlink.c >> @@ -3434,8 +3434,9 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow *flow, >> >> >> /* >> - * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out, >> - * then puts 'data' to 'buf'. >> + * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out. >> + * If the flow is not Ethernet, the OVS_KEY_ATTR_PACKET_TYPE is converted to >> + * OVS_KEY_ATTR_ETHERTYPE. Puts 'data' to 'buf'. >> */ >> static void >> put_exclude_packet_type(struct ofpbuf *buf, uint16_t type, >> @@ -3458,6 +3459,20 @@ put_exclude_packet_type(struct ofpbuf *buf, uint16_t type, >> ofs = nl_msg_start_nested(buf, type); >> nl_msg_put(buf, data, first_chunk_size); >> nl_msg_put(buf, next_attr, second_chunk_size); >> + if (!nl_attr_find__(data, data_len, OVS_KEY_ATTR_ETHERNET)) { >> + ovs_be16 pt = pt_ns_type_be(nl_attr_get_be32(packet_type)); >> + const struct nlattr *nla; >> + >> + nla = nl_attr_find(buf, NLA_HDRLEN, OVS_KEY_ATTR_ETHERTYPE); >> + if (nla) { >> + ovs_be16 *ethertype; >> + >> + ethertype = CONST_CAST(ovs_be16 *, nl_attr_get(nla)); >> + *ethertype = pt; >> + } else { >> + nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, pt); >> + } >> + } >> nl_msg_end_nested(buf, ofs); >> } else { >> nl_msg_put_unspec(buf, type, data, data_len); >> -- >> 2.11.1 >> > > Thanks Joe! This is definitely more concise than my version. > > Acked-by: Eric Garver <e@erig.me> Thanks for the review, applied to master.
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 63ef3de5c7d1..55effd1f91a8 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -3434,8 +3434,9 @@ dpif_netlink_flow_from_ofpbuf(struct dpif_netlink_flow *flow, /* - * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out, - * then puts 'data' to 'buf'. + * If PACKET_TYPE attribute is present in 'data', it filters PACKET_TYPE out. + * If the flow is not Ethernet, the OVS_KEY_ATTR_PACKET_TYPE is converted to + * OVS_KEY_ATTR_ETHERTYPE. Puts 'data' to 'buf'. */ static void put_exclude_packet_type(struct ofpbuf *buf, uint16_t type, @@ -3458,6 +3459,20 @@ put_exclude_packet_type(struct ofpbuf *buf, uint16_t type, ofs = nl_msg_start_nested(buf, type); nl_msg_put(buf, data, first_chunk_size); nl_msg_put(buf, next_attr, second_chunk_size); + if (!nl_attr_find__(data, data_len, OVS_KEY_ATTR_ETHERNET)) { + ovs_be16 pt = pt_ns_type_be(nl_attr_get_be32(packet_type)); + const struct nlattr *nla; + + nla = nl_attr_find(buf, NLA_HDRLEN, OVS_KEY_ATTR_ETHERTYPE); + if (nla) { + ovs_be16 *ethertype; + + ethertype = CONST_CAST(ovs_be16 *, nl_attr_get(nla)); + *ethertype = pt; + } else { + nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, pt); + } + } nl_msg_end_nested(buf, ofs); } else { nl_msg_put_unspec(buf, type, data, data_len);
For non-Ethernet flows, when fixing up the netlink message we need make sure to pass down a valid Ethertype. The kernel does not understand packet_type so it's implicitly encoded by the absence of _ETHERNET and exact match of _ETHERTYPE. Without this change match_validate in the kernel complains when trying to match packets from L3 tunnels. e.g. openvswitch: netlink: Unexpected mask (mask=110088, allowed=3d9804c) The mask use to always be set in xlate_wc_init() and xlate_wc_finish(), but that changed for non-Ethernet frames with the commit listed below. Fixes: 3d4b2e6eb74e ("userspace: Add OXM field MFF_PACKET_TYPE") Signed-off-by: Joe Stringer <joe@ovn.org> Co-authored-by: Eric Garver <e@erig.me> --- lib/dpif-netlink.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)