On Mon, Jul 10, 2017 at 09:30:22AM +0000, Zoltán Balogh wrote: > Hi Eric, > > Thank you for fixing this. I'm fine with your solution. Tests do pass.
Thanks for verifying. I'll add the patches below to my series and post it ASAP. > > Best regards, > Zoltan > > > This still did not do the trick. The ETHERTYPE is placed at the same > > level of the message as OVS_FLOW_ATTR_MASK, rather than being place > > _inside_ OVS_FLOW_ATTR_MASK. > > > > Below is what I came up with. All test cases pass unchanged. It also > > works for my series to add kernel L3 VXLAN-GPE/GRE. > > > > https://github.com/erig0/ovs/commits/46aafc3ad12e30772f9d8561e6750e6c6334077c > > > > Let me know how it works for you. > > > > Eric. > > > > --->8--- > > > > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > > index 562f6134c3a5..0654ba1b2131 100644 > > --- a/lib/dpif-netlink.c > > +++ b/lib/dpif-netlink.c > > @@ -3434,31 +3434,47 @@ 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, packet_type is converted to ETHERTYPE. > > + * Puts 'data' to 'buf'. > > */ > > static void > > put_exclude_packet_type(struct ofpbuf *buf, uint16_t type, > > - const struct nlattr *data, uint16_t data_len) > > + const struct nlattr *data, size_t data_len) > > { > > const struct nlattr *packet_type; > > > > packet_type = nl_attr_find__(data, data_len, OVS_KEY_ATTR_PACKET_TYPE); > > > > if (packet_type) { > > - /* exclude PACKET_TYPE Netlink attribute. */ > > - ovs_assert(NLA_ALIGN(packet_type->nla_len) == NL_A_U32_SIZE); > > - size_t packet_type_len = NL_A_U32_SIZE; > > - size_t first_chunk_size = (uint8_t *)packet_type - (uint8_t *)data; > > - size_t second_chunk_size = data_len - first_chunk_size > > - - packet_type_len; > > - uint8_t *first_attr = NULL; > > - struct nlattr *next_attr = nl_attr_next(packet_type); > > - > > - first_attr = nl_msg_put_unspec_uninit(buf, type, > > - data_len - packet_type_len); > > - memcpy(first_attr, data, first_chunk_size); > > - memcpy(first_attr + first_chunk_size, next_attr, > > second_chunk_size); > > + struct odputil_keybuf new_attrs_buf; > > + struct nlattr *new_attrs; > > + const struct nlattr *ethernet; > > + size_t new_attrs_len; > > + ovs_be32 packet_type_value = nl_attr_get_be32(packet_type); > > + > > + new_attrs = (struct nlattr *) &new_attrs_buf; > > + memcpy(new_attrs, data, data_len); > > + new_attrs_len = data_len; > > + > > + nl_attrs_filter(new_attrs, &new_attrs_len, > > OVS_KEY_ATTR_PACKET_TYPE); > > + > > + /* If it's not Ethernet, then we need to override or add the > > Ethertype > > + * with the one from the packet_type. */ > > + ethernet = nl_attr_find__(data, data_len, OVS_KEY_ATTR_ETHERNET); > > + if (!ethernet) { > > + struct ofpbuf new_buf; > > + > > + nl_attrs_filter(new_attrs, &new_attrs_len, > > OVS_KEY_ATTR_ETHERTYPE); > > + > > + ofpbuf_use_stack(&new_buf, new_attrs, sizeof new_attrs_buf); > > + nl_msg_put_uninit(&new_buf, new_attrs_len); > > + nl_msg_put_be16(&new_buf, OVS_KEY_ATTR_ETHERTYPE, > > + pt_ns_type_be(packet_type_value)); > > + new_attrs_len = new_buf.size; > > + } > > + > > + nl_msg_put_unspec(buf, type, new_attrs, new_attrs_len); > > } else { > > nl_msg_put_unspec(buf, type, data, data_len); > > } > > diff --git a/lib/netlink.c b/lib/netlink.c > > index 4cf1aaca621c..c9b83fc681c4 100644 > > --- a/lib/netlink.c > > +++ b/lib/netlink.c > > @@ -927,3 +927,23 @@ nl_attr_find_nested(const struct nlattr *nla, uint16_t > > type) > > { > > return nl_attr_find__(nl_attr_get(nla), nl_attr_get_size(nla), type); > > } > > + > > +/* > > + * Filter nlattr type from set of nlattrs. > > + * This changes the data in place. So caller should make a copy if > > required. > > + */ > > +void > > +nl_attrs_filter(struct nlattr *attrs, size_t *attrs_len, uint16_t type) > > +{ > > + size_t size = *attrs_len; > > + struct nlattr *nla; > > + size_t left; > > + > > + NL_ATTR_FOR_EACH (nla, left, attrs, size) { > > + if (nl_attr_type(nla) == type) { > > + *attrs_len -= nl_attr_len_pad(nla, left); > > + memmove(nla, nl_attr_next(nla), left - nl_attr_len_pad(nla, > > left)); > > + return; > > + } > > + } > > +} > > diff --git a/lib/netlink.h b/lib/netlink.h > > index 6dfac27c9d4b..744c9520966f 100644 > > --- a/lib/netlink.h > > +++ b/lib/netlink.h > > @@ -240,5 +240,6 @@ const struct nlattr *nl_attr_find(const struct ofpbuf > > *, size_t hdr_len, > > const struct nlattr *nl_attr_find_nested(const struct nlattr *, uint16_t > > type); > > const struct nlattr *nl_attr_find__(const struct nlattr *attrs, size_t > > size, > > uint16_t type); > > +void nl_attrs_filter(struct nlattr *attrs, size_t *attrs_len, uint16_t > > type); > > > > #endif /* netlink.h */ > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
