[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(&eth, 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,
                      &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