[ovs-dev] [PATCH v4 2/7] userspace: Support for push_eth and pop_eth actions
Yang, Yi Y
yi.y.yang at intel.com
Thu May 4 12:08:03 UTC 2017
Zoltan, original l3 userspace patch set gets ether_type from packet->md.
eh->eth_type = packet->md.packet_ethertype;
I think we should keep struct ovs_action_push_eth consistent in both kernel and userspace, we can use the way in original l3 userspace patch set to get ether_type.
-----Original Message-----
From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org] On Behalf Of Zoltán Balogh
Sent: Thursday, May 4, 2017 6:02 PM
To: Ben Pfaff <blp at ovn.org>; Jiri Benc (jbenc at redhat.com) <jbenc at redhat.com>
Cc: 'dev at openvswitch.org' <dev at openvswitch.org>
Subject: Re: [ovs-dev] [PATCH v4 2/7] userspace: Support for push_eth and pop_eth actions
> On Tue, Apr 25, 2017 at 04:30:23PM +0000, Zoltán Balogh wrote:
> > From: Jan Scheurich <jan.scheurich at ericsson.com>
> >
> > Add support for actions push_eth and pop_eth to the netdev datapath
> > and the supporting libraries. This patch relies on the support for
> > these actions in the kernel datapath to be present.
>
> Hi, can you help me to understand this part here? I believe that this
> will lead to userspace sending a OVS_ACTION_ATTR_PUSH_ETH action
> argument to the kernel that is different from what the kernel actually
> wants. What's the idea, and how is that problem avoided?
>
> struct ovs_action_push_eth {
> struct ovs_key_ethernet addresses;
> #ifndef __KERNEL__
> __be16 eth_type;
> #endif
> };
>
> Thanks,
>
> Ben.
Hi,
I'm afraid the kernel would not accept that OVS_ACTION_ATTR_PUSH_ETH action in this case.
Jiri, could you confirm, please?
We could remove the eth_type from struct ovs_action_push_eth, then put an additional "set action" after putting "push_eth" in the odp_put_push_eth_action() function in order to set the ethertype of the packet.
void
odp_put_push_eth_action(struct ofpbuf *odp_actions,
const struct eth_addr *eth_src,
const struct eth_addr *eth_dst,
const ovs_be16 eth_type) {
struct ovs_action_push_eth eth;
memset(ð, 0, sizeof eth);
if (eth_src) {
eth.addresses.eth_src = *eth_src;
}
if (eth_dst) {
eth.addresses.eth_dst = *eth_dst;
}
nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_PUSH_ETH,
ð, sizeof eth);
commit_set_action(odp_actions, OVS_KEY_ATTR_ETHERTYPE, eth_type); }
That way we would split the push_eth action into a simpler push_eth and a set_field. However this would lead to modify odp_execute_set_action()as well, since changing of ethertype with set_field is not allowed:
static void
odp_execute_set_action(struct dp_packet *packet, const struct nlattr *a) {
enum ovs_key_attr type = nl_attr_type(a);
const struct ovs_key_ipv4 *ipv4_key;
const struct ovs_key_ipv6 *ipv6_key;
struct pkt_metadata *md = &packet->md;
switch (type) {
...
case OVS_KEY_ATTR_ETHERTYPE:
case OVS_KEY_ATTR_IN_PORT:
case OVS_KEY_ATTR_VLAN:
case OVS_KEY_ATTR_TCP_FLAGS:
case OVS_KEY_ATTR_CT_STATE:
case OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4:
case OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6:
case OVS_KEY_ATTR_CT_ZONE:
case OVS_KEY_ATTR_CT_MARK:
case OVS_KEY_ATTR_CT_LABELS:
case __OVS_KEY_ATTR_MAX:
default:
OVS_NOT_REACHED();
}
}
I guess something similar should be done at kernel side too, since the kernel module would not accept set_field ethertype either.
Another option would be to convert OVS_ACTION_ATTR_PUSH_ETH action argument between userspace and kernel.
Do you have any other proposal?
Btw, the original comment message of struct ovs_action_push_eth indicates eth_type as a second member.
/*
* struct ovs_action_push_eth - %OVS_ACTION_ATTR_PUSH_ETH action argument.
* @addresses: Source and destination MAC addresses.
* @eth_type: Ethernet type
*/
struct ovs_action_push_eth {
struct ovs_key_ethernet addresses;
};
Best regards,
Zoltan
_______________________________________________
dev mailing list
dev at openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list