[ovs-dev] [PATCH 1/4] datapath: Add basic MPLS support to kernel

Isaku Yamahata yamahata at valinux.co.jp
Fri Oct 26 08:08:31 UTC 2012


Some inlined comments in case label == 0. I don't find any dependency on it.

On Thu, Oct 18, 2012 at 06:02:18PM +0900, Simon Horman wrote:
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index a6915fb..e606ae1 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -74,6 +74,43 @@ int ovs_net_id __read_mostly;
>  int (*ovs_dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd);
>  EXPORT_SYMBOL(ovs_dp_ioctl_hook);
>  
> +void skb_cb_set_mpls_bos(struct sk_buff *skb)
> +{
> +	struct ethhdr *eth;
> +	int nh_ofs;
> +	__be16 dl_type = 0;
> +
> +	skb_reset_mac_header(skb);
> +
> +	eth = eth_hdr(skb);
> +	nh_ofs = sizeof(struct ethhdr);
> +	if (likely(eth->h_proto >= htons(ETH_TYPE_MIN))) {
> +		dl_type = eth->h_proto;
> +
> +		while (dl_type == htons(ETH_P_8021Q) &&
> +				skb->len >= nh_ofs + sizeof(struct vlan_hdr)) {
> +			struct vlan_hdr *vh = (struct vlan_hdr*)(skb->data + nh_ofs);
> +			dl_type = vh->h_vlan_encapsulated_proto;
> +			nh_ofs += sizeof(struct vlan_hdr);
> +		}
> +
> +		OVS_CB(skb)->mpls_bos = nh_ofs;
> +	} else {
> +		OVS_CB(skb)->mpls_bos = 0;
> +	}
> +}
> +
> +unsigned char *skb_cb_mpls_bos(const struct sk_buff *skb)
> +{
> +	return OVS_CB(skb)->mpls_bos ?
> +		skb_mac_header(skb) + OVS_CB(skb)->mpls_bos : 0;
> +}
> +
> +ptrdiff_t skb_cb_mpls_bos_offset(const struct sk_buff *skb)
> +{
> +	return OVS_CB(skb)->mpls_bos;
> +}
> +
>  /**
>   * DOC: Locking:
>   *
> @@ -663,6 +700,9 @@ static int validate_actions(const struct nlattr *attr,
>  		static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
>  			[OVS_ACTION_ATTR_OUTPUT] = sizeof(u32),
>  			[OVS_ACTION_ATTR_USERSPACE] = (u32)-1,
> +			[OVS_ACTION_ATTR_PUSH_MPLS] = sizeof(struct ovs_action_push_mpls),
> +			[OVS_ACTION_ATTR_POP_MPLS] = sizeof(__be16),
> +			[OVS_ACTION_ATTR_SET_MPLS] = sizeof(__be32),
>  			[OVS_ACTION_ATTR_PUSH_VLAN] = sizeof(struct ovs_action_push_vlan),
>  			[OVS_ACTION_ATTR_POP_VLAN] = 0,
>  			[OVS_ACTION_ATTR_SET] = (u32)-1,
> @@ -691,6 +731,24 @@ static int validate_actions(const struct nlattr *attr,
>  				return -EINVAL;
>  			break;
>  
> +		case OVS_ACTION_ATTR_PUSH_MPLS: {
> +			const struct ovs_action_push_mpls *mpls = nla_data(a);
> +			if (mpls->mpls_ethertype != htons(ETH_P_MPLS_UC) &&
> +			    mpls->mpls_ethertype != htons(ETH_P_MPLS_MC))
> +				return -EINVAL;
> +			break;
> +		}
> +
> +		case OVS_ACTION_ATTR_POP_MPLS:
> +			break;
> +
> +		case OVS_ACTION_ATTR_SET_MPLS: {
> +			__be32 mpls_label = nla_get_be32(a);
> +			if (mpls_label == htonl(0)) {
> +				return -EINVAL;
> +			}

Is this necessary?
At least, this is inconsistent with case OVS_ACTION_ATTR_PUSH_MPLS.


> +			break;
> +		}
>  
>  		case OVS_ACTION_ATTR_POP_VLAN:
>  			break;
> @@ -805,6 +863,8 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
>  	OVS_CB(packet)->flow = flow;
>  	packet->priority = flow->key.phy.priority;
>  
> +	skb_cb_set_mpls_bos(packet);
> +
>  	rcu_read_lock();
>  	dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>  	err = -ENODEV;


... snip ...

> diff --git a/datapath/flow.h b/datapath/flow.h
> index 02c563a..eb3fe49 100644
> --- a/datapath/flow.h
> +++ b/datapath/flow.h
> @@ -53,6 +53,9 @@ struct sw_flow_key {
>  		__be16 type;		/* Ethernet frame type. */
>  	} eth;
>  	struct {
> +		__be32 top_label;	/* 0 if no MPLS, top label from stack otherwise */

I think that the validity of top_label is guaranteed by ethertype = MPLS.
So top_label can be 0 as valid label value.


> +	} mpls;
> +	struct {
>  		u8     proto;		/* IP protocol or lower 8 bits of ARP opcode. */
>  		u8     tos;		/* IP ToS. */
>  		u8     ttl;		/* IP TTL/hop limit. */
> @@ -126,6 +129,10 @@ struct arp_eth_header {
>  	unsigned char       ar_tip[4];		/* target IP address        */
>  } __packed;
>  
> +#define ETH_TYPE_MIN 0x600
> +
> +#define MPLS_HLEN 4
> +
>  int ovs_flow_init(void);
>  void ovs_flow_exit(void);
>  

-- 
yamahata



More information about the dev mailing list