>
> A new concern came up while thinking about this series. The
> OVS_ATTR_PACKET_TYPE does not appear to be implemented in the kernel
> module, and what's more, because of #ifdefs, OVS_ATTR_PACKET_TYPE will
> actually have a different value in the kernel module than in userspace.
> What's the plan here?
Hi Ben,
OVS_KEY_ATTR_PACKET_TYPE is represented with OVS_KEY_ATTR_ETHERNET and
OVS_KEY_ATTR_ETHERTYPE in the kernel.
>From Google doc:
"The presence of netlink key attribute OVS_KEY_ATTR_ETHERNET is used to
indicate if it's about L2 or L3 packet on the netlink interface. The "L3"
packet type is encoded in the OVS_KEY_ATTR_ETHERTYPE netlink attribute."
The plan was to do a conversion between OVS_KEY_ATTR_PACKET_TYPE and the pair
of OVS_KEY_ATTR_ETHERNET + OVS_KEY_ATTR_ETHERTYPE attributes before/after
transmission over netlink. Currently the PACKET_TYPE attribute is filtered out
in the put_exclude_packet_type() function in lib/dpif-netlink.c.
put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
const struct nlattr *data, uint16_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);
} else {
nl_msg_put_unspec(buf, type, data, data_len);
}
}
This could be modified to do more verification and put OVS_KEY_ATTR_ETHERTYPE
if needed:
---
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 562f6134c..bdcc76c7b 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -3434,11 +3434,11 @@ 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', converts it to ETHERNET and
+ * ETHERTYPE attributes, then puts 'data' to 'buf'.
*/
static void
-put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
+put_convert_packet_type(struct ofpbuf *buf, uint16_t type,
const struct nlattr *data, uint16_t data_len)
{
const struct nlattr *packet_type;
@@ -3446,8 +3446,9 @@ put_exclude_packet_type(struct ofpbuf *buf, uint16_t type,
packet_type = nl_attr_find__(data, data_len, OVS_KEY_ATTR_PACKET_TYPE);
if (packet_type) {
- /* exclude PACKET_TYPE Netlink attribute. */
+ /* convert PACKET_TYPE Netlink attribute. */
ovs_assert(NLA_ALIGN(packet_type->nla_len) == NL_A_U32_SIZE);
+ ovs_be32 value = nl_attr_get_be32(packet_type);
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
@@ -3455,10 +3456,31 @@ put_exclude_packet_type(struct ofpbuf *buf, uint16_t
type,
uint8_t *first_attr = NULL;
struct nlattr *next_attr = nl_attr_next(packet_type);
+ bool ethernet_present = nl_attr_find__(data, data_len,
+ OVS_KEY_ATTR_ETHERNET);
+
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);
+
+ /* Put OVS_KEY_ATTR_ETHERTYPE if needed. */
+ if (ntohl(value) == PT_ETH) {
+ ovs_assert(ethernet_present);
+ } else {
+ const struct nlattr *eth_type;
+ ovs_be16 ns_type = pt_ns_type_be(value);
+
+ ovs_assert(ethernet_present == false);
+
+ eth_type = nl_attr_find__(data, data_len, OVS_KEY_ATTR_ETHERTYPE);
+ if (eth_type) {
+ ovs_assert(nl_attr_get_be16(eth_type) == ns_type);
+ } else {
+ nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, ns_type);
+ }
+ }
+
} else {
nl_msg_put_unspec(buf, type, data, data_len);
}
@@ -3489,11 +3511,11 @@ dpif_netlink_flow_to_ofpbuf(const struct
dpif_netlink_flow *flow,
}
if (!flow->ufid_terse || !flow->ufid_present) {
if (flow->key_len) {
- put_exclude_packet_type(buf, OVS_FLOW_ATTR_KEY, flow->key,
+ put_convert_packet_type(buf, OVS_FLOW_ATTR_KEY, flow->key,
flow->key_len);
}
if (flow->mask_len) {
- put_exclude_packet_type(buf, OVS_FLOW_ATTR_MASK, flow->mask,
+ put_convert_packet_type(buf, OVS_FLOW_ATTR_MASK, flow->mask,
flow->mask_len);
}
if (flow->actions || flow->actions_len) {
---
On the Rx side, upcall->packet.packet_type is set in the parse_odp_packet()
called by dpif_netlink_recv__().
I'm not sure, if this is enough. Could someone double check, please?
Best regards,
Zoltan
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
