[ovs-dev] [PATCH 2/4] user-space datapath: Add basic MPLS support to kernel
This is to match the kernel implementation in the
patch "datapath: Add basic MPLS support to kernel"
by Leo Alterman.
Cc: Leo Alterman
Signed-off-by: Simon Horman
---
v2.2
* No change
v2.1
* Initial post
---
lib/dpif-netdev.c | 13 +
lib/odp-util.c| 43 +++
2 files changed, 56 insertions(+)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c9e3210..6deb9ff 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1212,6 +1212,7 @@ execute_set_action(struct ofpbuf *packet, const struct
nlattr *a)
case OVS_KEY_ATTR_ICMPV6:
case OVS_KEY_ATTR_ARP:
case OVS_KEY_ATTR_ND:
+ case OVS_KEY_ATTR_MPLS:
case __OVS_KEY_ATTR_MAX:
default:
NOT_REACHED();
@@ -1249,6 +1250,18 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
eth_pop_vlan(packet);
break;
+case OVS_ACTION_ATTR_PUSH_MPLS:
+push_mpls(packet, nl_attr_get_be16(a));
+break;
+
+case OVS_ACTION_ATTR_POP_MPLS:
+ pop_mpls(packet, nl_attr_get_be16(a));
+ break;
+
+case OVS_ACTION_ATTR_SET_MPLS:
+ set_mpls_lse(packet, nl_attr_get_be32(a));
+ break;
+
case OVS_ACTION_ATTR_SET:
execute_set_action(packet, nl_attr_get(a));
break;
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 257d7a7..13b3248 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -73,6 +73,9 @@ odp_action_len(uint16_t type)
case OVS_ACTION_ATTR_USERSPACE: return -2;
case OVS_ACTION_ATTR_PUSH_VLAN: return sizeof(struct ovs_action_push_vlan);
case OVS_ACTION_ATTR_POP_VLAN: return 0;
+case OVS_ACTION_ATTR_PUSH_MPLS: return sizeof(struct ovs_action_push_mpls);
+case OVS_ACTION_ATTR_POP_MPLS: return sizeof(ovs_be16);
+case OVS_ACTION_ATTR_SET_MPLS: return sizeof(ovs_be32);
case OVS_ACTION_ATTR_SET: return -2;
case OVS_ACTION_ATTR_SAMPLE: return -2;
@@ -106,6 +109,7 @@ ovs_key_attr_to_string(enum ovs_key_attr attr)
case OVS_KEY_ATTR_ARP: return "arp";
case OVS_KEY_ATTR_ND: return "nd";
case OVS_KEY_ATTR_TUN_ID: return "tun_id";
+case OVS_KEY_ATTR_MPLS: return "mpls";
case __OVS_KEY_ATTR_MAX:
default:
@@ -273,6 +277,17 @@ format_vlan_tci(struct ds *ds, ovs_be16 vlan_tci)
}
static void
+format_mpls_lse(struct ds *ds, ovs_be32 mpls_lse)
+{
+ds_put_format(ds, "label=%"PRIu32",tc=%d,ttl=%d,bos=%d",
+ mpls_lse_to_label(mpls_lse),
+ mpls_lse_to_tc(mpls_lse),
+ mpls_lse_to_ttl(mpls_lse),
+ mpls_lse_to_stack(mpls_lse));
+}
+
+
+static void
format_odp_action(struct ds *ds, const struct nlattr *a)
{
int expected_len;
@@ -311,6 +326,25 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
case OVS_ACTION_ATTR_POP_VLAN:
ds_put_cstr(ds, "pop_vlan");
break;
+case OVS_ACTION_ATTR_PUSH_MPLS: {
+const struct ovs_action_push_mpls *mpls = nl_attr_get(a);
+ds_put_cstr(ds, "push_mpls(");
+format_mpls_lse(ds, mpls->mpls_label);
+ds_put_format(ds, "eth_type=0x%"PRIx16")",
ntohs(mpls->mpls_ethertype));
+break;
+}
+case OVS_ACTION_ATTR_POP_MPLS: {
+ovs_be16 ethertype = nl_attr_get_be16(a);
+ds_put_format(ds, "pop_mpls(eth_type=0x%"PRIx16")", ntohs(ethertype));
+break;
+ }
+case OVS_ACTION_ATTR_SET_MPLS: {
+ovs_be32 label = nl_attr_get_be32(a);
+ds_put_cstr(ds, "set_mpls(");
+format_mpls_lse(ds, label);
+ds_put_char(ds, ')');
+break;
+}
case OVS_ACTION_ATTR_SAMPLE:
format_odp_sample_action(ds, a);
break;
@@ -606,6 +640,7 @@ odp_flow_key_attr_len(uint16_t type)
case OVS_KEY_ATTR_ETHERNET: return sizeof(struct ovs_key_ethernet);
case OVS_KEY_ATTR_VLAN: return sizeof(ovs_be16);
case OVS_KEY_ATTR_ETHERTYPE: return 2;
+case OVS_KEY_ATTR_MPLS: return sizeof(struct ovs_key_mpls);
case OVS_KEY_ATTR_IPV4: return sizeof(struct ovs_key_ipv4);
case OVS_KEY_ATTR_IPV6: return sizeof(struct ovs_key_ipv6);
case OVS_KEY_ATTR_TCP: return sizeof(struct ovs_key_tcp);
@@ -715,6 +750,14 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
ds_put_char(ds, ')');
break;
+case OVS_KEY_ATTR_MPLS: {
+const struct ovs_key_mpls *mpls_key = nl_attr_get(a);
+ds_put_char(ds, '(');
+format_mpls_lse(ds, mpls_key->mpls_top_label);
+ds_put_char(ds, ')');
+break;
+}
+
case OVS_KEY_ATTR_ETHERTYPE:
ds_put_format(ds, "(0x%04"PRIx16")",
ntohs(nl_attr_get_be16(a)));
--
1.7.10.4
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 3/4] nx-match: Do not check pre-requisites for load actions
There are (or at least will be) cases where this check can produce false
positives. For example, a flow which matches a non-MPLS packet and then
applies an MPLS push action followed by an action to load the MPLS label.
I believe this is the same problem that was recently discussed in
relation to set-field. In that case my understanding is that consensus
was to omit the check and potentially revisit the problem later.
Signed-off-by: Simon Horman
---
v2.2
* No change
v2.1
* Initial post
---
lib/autopath.c|6 +++---
lib/autopath.h|3 +--
lib/bundle.c |7 +++
lib/bundle.h |3 +--
lib/learn.c | 10 +-
lib/learn.h |2 +-
lib/meta-flow.c | 14 +-
lib/meta-flow.h |4 ++--
lib/multipath.c |7 +++
lib/multipath.h |3 +--
lib/nx-match.c| 16
lib/nx-match.h|6 ++
lib/ofp-actions.c | 23 +++
lib/ofp-actions.h |2 +-
ofproto/ofproto.c |4 ++--
15 files changed, 49 insertions(+), 61 deletions(-)
diff --git a/lib/autopath.c b/lib/autopath.c
index b204e84..1af5cd9 100644
--- a/lib/autopath.c
+++ b/lib/autopath.c
@@ -80,16 +80,16 @@ autopath_from_openflow(const struct nx_action_autopath *nap,
return OFPERR_OFPBAC_BAD_ARGUMENT;
}
-return autopath_check(autopath, NULL);
+return autopath_check(autopath);
}
enum ofperr
-autopath_check(const struct ofpact_autopath *autopath, const struct flow *flow)
+autopath_check(const struct ofpact_autopath *autopath)
{
VLOG_WARN_ONCE("The autopath action is deprecated and may be removed in"
" February 2013. Please email dev@openvswitch.org with"
" concerns.");
-return mf_check_dst(&autopath->dst, flow);
+return mf_check_dst(&autopath->dst);
}
void
diff --git a/lib/autopath.h b/lib/autopath.h
index 337e7d1..ef46456 100644
--- a/lib/autopath.h
+++ b/lib/autopath.h
@@ -33,8 +33,7 @@ void autopath_parse(struct ofpact_autopath *, const char *);
enum ofperr autopath_from_openflow(const struct nx_action_autopath *,
struct ofpact_autopath *);
-enum ofperr autopath_check(const struct ofpact_autopath *,
- const struct flow *);
+enum ofperr autopath_check(const struct ofpact_autopath *);
void autopath_to_nxast(const struct ofpact_autopath *,
struct ofpbuf *openflow);
diff --git a/lib/bundle.c b/lib/bundle.c
index b68ebab..6ced9ad 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -173,20 +173,19 @@ bundle_from_openflow(const struct nx_action_bundle *nab,
ofpact_update_len(ofpacts, &bundle->ofpact);
if (!error) {
-error = bundle_check(bundle, OFPP_MAX, NULL);
+error = bundle_check(bundle, OFPP_MAX);
}
return error;
}
enum ofperr
-bundle_check(const struct ofpact_bundle *bundle, int max_ports,
- const struct flow *flow)
+bundle_check(const struct ofpact_bundle *bundle, int max_ports)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
size_t i;
if (bundle->dst.field) {
-enum ofperr error = mf_check_dst(&bundle->dst, flow);
+enum ofperr error = mf_check_dst(&bundle->dst);
if (error) {
return error;
}
diff --git a/lib/bundle.h b/lib/bundle.h
index 5b6bb67..a41f3f6 100644
--- a/lib/bundle.h
+++ b/lib/bundle.h
@@ -39,8 +39,7 @@ uint16_t bundle_execute(const struct ofpact_bundle *, const
struct flow *,
void *aux);
enum ofperr bundle_from_openflow(const struct nx_action_bundle *,
struct ofpbuf *ofpact);
-enum ofperr bundle_check(const struct ofpact_bundle *, int max_ports,
- const struct flow *);
+enum ofperr bundle_check(const struct ofpact_bundle *, int max_ports);
void bundle_to_nxast(const struct ofpact_bundle *, struct ofpbuf *of10);
void bundle_parse(const char *, struct ofpbuf *ofpacts);
void bundle_parse_load(const char *, struct ofpbuf *ofpacts);
diff --git a/lib/learn.c b/lib/learn.c
index b9bbc97..f9d5527 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -168,7 +168,7 @@ learn_from_openflow(const struct nx_action_learn *nal,
struct ofpbuf *ofpacts)
/* Checks that 'learn' is a valid action on 'flow'. Returns 0 if it is valid,
* otherwise an OFPERR_*. */
enum ofperr
-learn_check(const struct ofpact_learn *learn, const struct flow *flow)
+learn_check(const struct ofpact_learn *learn)
{
const struct ofpact_learn_spec *spec;
struct match match;
@@ -179,7 +179,7 @@ learn_check(const struct ofpact_learn *learn, const struct
flow *flow)
/* Check the source. */
if (spec->src_type == NX_LEARN_SRC_FIELD) {
-error = mf_check_src(&spec->src, flow);
+error = mf_check_src(&spec->src);
if (error) {
return error;
}
@@ -188,7 +188,7 @@ learn_check(const str
[ovs-dev] [PATCH 1/4] datapath: Add basic MPLS support to kernel
Allow datapath to recognize and extract MPLS labels into flow keys
and execute actions which push, pop, and set labels on packets.
Based heavily on work by Leo Alterman.
Cc: Leo Alterman
Signed-off-by: Simon Horman
---
v2.2
* Call skb_reset_mac_header() in skb_cb_set_mpls_stack()
eth_hdr(skb) is non-NULL when called in skb_cb_set_mpls_stack().
* Add a call to skb_cb_set_mpls_stack() in ovs_packet_cmd_execute().
I apologise that I have mislaid my notes on this but
it avoids a kernel panic. I can investigate again if necessary.
* Use struct ovs_action_push_mpls instead of
__be16 to decode OVS_ACTION_ATTR_PUSH_MPLS in validate_actions(). This is
consistent with the data format for the attribute.
* Indentation fix in skb_cb_mpls_stack(). [cosmetic]
v2.1
* Manual rebase
Previous version(s) by Leo Alterman
---
datapath/actions.c | 81 +++
datapath/datapath.c | 61
datapath/datapath.h |8 +
datapath/flow.c | 30
datapath/flow.h |7
datapath/vport.c|2 ++
include/linux/openvswitch.h | 32 +
7 files changed, 221 insertions(+)
diff --git a/datapath/actions.c b/datapath/actions.c
index ec9b595..c1d1a7f 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -47,6 +47,67 @@ static int make_writable(struct sk_buff *skb, int write_len)
return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
}
+static __be16 get_ethertype(const struct sk_buff *skb)
+{
+ struct ethhdr *hdr = (struct ethhdr *)(skb_cb_mpls_stack(skb) -
ETH_HLEN);
+ return hdr->h_proto;
+}
+
+static void set_ethertype(struct sk_buff *skb, const __be16 ethertype)
+{
+ struct ethhdr *hdr = (struct ethhdr *)(skb_cb_mpls_stack(skb) -
ETH_HLEN);
+ hdr->h_proto = ethertype;
+}
+
+static int push_mpls(struct sk_buff *skb, const struct ovs_action_push_mpls
*mpls)
+{
+ u32 l2_size;
+ __be32 *new_mpls_label;
+
+ if (skb_cow_head(skb, MPLS_HLEN) < 0) {
+ kfree_skb(skb);
+ return -ENOMEM;
+ }
+
+ l2_size = skb_cb_mpls_stack_offset(skb);
+ skb_push(skb, MPLS_HLEN);
+ memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb), l2_size);
+ skb_reset_mac_header(skb);
+
+ new_mpls_label = (__be32 *)(skb_mac_header(skb) + l2_size);
+ *new_mpls_label = mpls->mpls_label;
+
+ set_ethertype(skb, mpls->mpls_ethertype);
+ return 0;
+}
+
+static int pop_mpls(struct sk_buff *skb, const __be16 *ethertype)
+{
+ __be16 current_ethertype = get_ethertype(skb);
+ if (current_ethertype == htons(ETH_P_MPLS_UC) ||
+ current_ethertype == htons(ETH_P_MPLS_MC)) {
+ u32 l2_size = skb_cb_mpls_stack_offset(skb);
+
+ memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
l2_size);
+
+ skb_pull(skb, MPLS_HLEN);
+ skb_reset_mac_header(skb);
+
+ set_ethertype(skb, *ethertype);
+ }
+ return 0;
+}
+
+static int set_mpls(struct sk_buff *skb, const __be32 *mpls_label)
+{
+ __be16 current_ethertype = get_ethertype(skb);
+ if (current_ethertype == htons(ETH_P_MPLS_UC) ||
+ current_ethertype == htons(ETH_P_MPLS_MC)) {
+ memcpy(skb_cb_mpls_stack(skb), mpls_label, sizeof(__be32));
+ }
+ return 0;
+}
+
/* remove VLAN header from packet and update csum accordingly. */
static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
{
@@ -100,6 +161,9 @@ static int pop_vlan(struct sk_buff *skb)
return err;
__vlan_hwaccel_put_tag(skb, ntohs(tci));
+
+ /* update pointer to MPLS label stack */
+ skb_cb_set_mpls_stack(skb);
return 0;
}
@@ -120,6 +184,9 @@ static int push_vlan(struct sk_buff *skb, const struct
ovs_action_push_vlan *vla
}
__vlan_hwaccel_put_tag(skb, ntohs(vlan->vlan_tci) & ~VLAN_TAG_PRESENT);
+
+ /* update pointer to MPLS label stack */
+ skb_cb_set_mpls_stack(skb);
return 0;
}
@@ -396,6 +463,20 @@ static int do_execute_actions(struct datapath *dp, struct
sk_buff *skb,
output_userspace(dp, skb, a);
break;
+ case OVS_ACTION_ATTR_PUSH_MPLS:
+ err = push_mpls(skb, nla_data(a));
+ if (unlikely(err)) /* skb already freed. */
+ return err;
+ break;
+
+ case OVS_ACTION_ATTR_POP_MPLS:
+ err = pop_mpls(skb, nla_data(a));
+ break;
+
+ case OVS_ACTION_ATTR_SET_MPLS:
+ err = set_mpls(skb, nla_data(a));
+ break;
+
case OVS_ACTION_ATTR_PUSH_VLAN:
err = push_vlan(skb, nla_data(a));
[ovs-dev] [PATCH 0/4 v2.2] Basic MPLS actions and matches
Hi, This series implements basic MPLS actions and matches based on the kernel datapath work by Leo Alterman. Some limitations in the scope of this series are described in the changelog entry for the last patch in the series. The key differences between this series and the previous version (v2.1) are: * Squashed changes to kernel-datapath into first patch of series * Non-trivial rebase on top of master Git and diffstat information is provided below to aid review. The following changes since commit 1d446463ea3707ee609ea45f0cd2714aa7f5bfc6: ofproto-dpif: Avoid zeroing tunnel info in handle_miss_upcalls(). (2012-10-04 16:57:54 -0700) are available in the git repository at: git://github.com/horms/openvswitch.git devel/mpls for you to fetch changes up to 1af93db1f2efaf1591510f86419b3517e34aa2e2: User-Space MPLS actions and matches (2012-10-09 16:07:15 +0900) Simon Horman (4): datapath: Add basic MPLS support to kernel user-space datapath: Add basic MPLS support to kernel nx-match: Do not check pre-requisites for load actions User-Space MPLS actions and matches datapath/actions.c | 81 + datapath/datapath.c | 61 +++ datapath/datapath.h |8 + datapath/flow.c | 30 datapath/flow.h |7 + datapath/vport.c|2 + include/linux/openvswitch.h | 32 include/openflow/nicira-ext.h | 84 + include/openflow/openflow-1.2.h |2 + lib/autopath.c |6 +- lib/autopath.h |3 +- lib/bundle.c|7 +- lib/bundle.h|3 +- lib/dpif-netdev.c | 13 ++ lib/flow.c | 120 - lib/flow.h | 14 +- lib/learn.c | 10 +- lib/learn.h |2 +- lib/match.c | 69 +++- lib/match.h |6 + lib/meta-flow.c | 138 ++- lib/meta-flow.h | 13 +- lib/multipath.c |7 +- lib/multipath.h |3 +- lib/nx-match.c | 38 - lib/nx-match.h |6 +- lib/odp-util.c | 194 - lib/ofp-actions.c | 107 ++-- lib/ofp-actions.h | 20 ++- lib/ofp-parse.c | 14 ++ lib/ofp-print.c |4 + lib/ofp-util.c | 28 ++- lib/ofp-util.def|4 + lib/ofpbuf.c|8 +- lib/ofpbuf.h|1 + lib/packets.c | 356 +++ lib/packets.h | 88 ++ ofproto/ofproto-dpif.c | 96 ++- ofproto/ofproto.c |4 +- tests/automake.mk |5 + tests/odp.at| 16 +- tests/ofp-print.at |6 +- tests/ofproto-dpif.at | 141 +--- tests/ofproto.at| 12 +- tests/test-mpls.c | 288 +++ utilities/ovs-dpctl.c | 18 +- utilities/ovs-ofctl.8.in| 21 +++ 47 files changed, 2064 insertions(+), 132 deletions(-) create mode 100644 tests/test-mpls.c ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVS and namespaces
On 10/08/2012 10:24 PM, Jesse Gross wrote: On Sat, Oct 6, 2012 at 10:21 AM, Gary Kotton wrote: Hi, When Quantum is using OVS and namespaces there are problems when namespace is being deleted. More specifically this is when a default gateway is assigned to an interface that is used by the OVS. A simple way of reproducing this is: sudo ip netns exec ip route del ... sudo ip netns exec ovs-vsctl del-port... From this moment on one is not able to access the namespace. The problem does not occur when the traditional linux bridges are used. Has anyone encountered this before? I haven't heard of it before and can't think of any reasons why it might be the case. I tried reproducing it as well but couldn't although I'm not sure that I understand what you mean by not being able to access the namespace. Can you give a more detailed set of steps of how to reproduce this and more information about your environment (like the kernel version)? Hi, Sorry for being a bit vague in the description. The problem occurs when running OVS with Quantum and namespaces are enabled (this is the default). It is very easy to reproduce. I have done this one Ubuntu 12.04, Fedora 17 and Fedora 18. The easiest way of reproducing is to make use of devstack. (www.devstack.org) Reproduction steps: 1. git clone git://github.com/openstack-dev/devstack.git 2. Add the following to the file - stackrc ENABLED_SERVICES+=q-svc,quantum,q-agt,q-dhcp,q-l3 Q_PLUGIN=openvswitch 3. ./stack.sh This launches openstack with OVS. The installtion will create 2 namespaces. The first for the dhcp service and the second for the layer 3 agent. You can then run ' nova boot --image cirros-0.3.0-x86_64-uec --flavor 1 t1' to launch a VM. Once this is done you can try and delete the namespaces and you will see the problem. Alternativey you can take a look at - https://fedoraproject.org/wiki/Test_Day:2012-09-18_OpenStack and the link https://fedoraproject.org/wiki/QA:Testcase_Quantum_V2 Thanks Gary ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVS and namespaces
On Tue, Oct 9, 2012 at 4:25 AM, Gary Kotton wrote: > On 10/08/2012 10:24 PM, Jesse Gross wrote: >> >> On Sat, Oct 6, 2012 at 10:21 AM, Gary Kotton wrote: >>> >>> Hi, >>> When Quantum is using OVS and namespaces there are problems when >>> namespace >>> is being deleted. More specifically this is when a default gateway is >>> assigned to an interface that is used by the OVS. A simple way of >>> reproducing this is: >>> >>> sudo ip netns exec ip route del ... >>> sudo ip netns exec ovs-vsctl del-port... >>> >>> From this moment on one is not able to access the namespace. The problem >>> does not occur when the traditional linux bridges are used. >>> >>> Has anyone encountered this before? >> >> I haven't heard of it before and can't think of any reasons why it >> might be the case. I tried reproducing it as well but couldn't >> although I'm not sure that I understand what you mean by not being >> able to access the namespace. >> >> Can you give a more detailed set of steps of how to reproduce this and >> more information about your environment (like the kernel version)? > > Hi, > Sorry for being a bit vague in the description. The problem occurs when > running OVS with Quantum and namespaces are enabled (this is the default). > It is very easy to reproduce. I have done this one Ubuntu 12.04, Fedora 17 > and Fedora 18. The easiest way of reproducing is to make use of devstack. > (www.devstack.org) > Reproduction steps: > 1. git clone git://github.com/openstack-dev/devstack.git > 2. Add the following to the file - stackrc > ENABLED_SERVICES+=q-svc,quantum,q-agt,q-dhcp,q-l3 > Q_PLUGIN=openvswitch > 3. ./stack.sh > This launches openstack with OVS. The installtion will create 2 namespaces. > The first for the dhcp service and the second for the layer 3 agent. > You can then run ' nova boot --image cirros-0.3.0-x86_64-uec --flavor 1 t1' > to launch a VM. > Once this is done you can try and delete the namespaces and you will see the > problem. > > Alternativey you can take a look at - > https://fedoraproject.org/wiki/Test_Day:2012-09-18_OpenStack and the link > https://fedoraproject.org/wiki/QA:Testcase_Quantum_V2 Can you please try to narrow the set of steps down to those that actually involve OVS without OpenStack? I'm having a hard time imagining something in OVS causing the problem that you're describing (and I couldn't reproduce it) so my suspicion is that it's something in the way that OpenStack uses OVS rather than OVS itself. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 4/4] User-Space MPLS actions and matches
The change to execute_set_action(), it implies that OVS_KEY_ATTR_MPLS
got put in a place you don't want it in a previous patch:
parse_remaining_mpls() will read past the end of the packet.
I don't think that flow_extract() checks that the packet is long
enough for an IPv4 or IPv6 header when it sets nw_ttl.
In flow_format(), I think it would be better to output nothing rather
than mpls(0) if there is no MPLS header.
s/encapsualted/encapsulated/ in parse_mpls_onward().
In flow_compose(), I would normally format
inner_dl_type = flow->encap_dl_type == htons(0)
? flow->dl_type
: flow->encap_dl_type;
as
inner_dl_type = (flow->encap_dl_type == htons(0)
? flow->dl_type
: flow->encap_dl_type);
In flow_compose(), I think that the tests like this:
if (flow->dl_type == htons(ETH_TYPE_IP) ||
flow->encap_dl_type == htons(ETH_TYPE_IP)) {
can be written to just check inner_dl_type instead.
In match_set_any_mpls_label(), I think that |= should be &=.
Similarly for the other mpls_set_any_*() functions.
I no longer think that we should add NXM constants for the MPLS
fields. I think when I asked Ravi to do that, we didn't have OXM
support. Instead, let's just use the OXM constants (OXM_OF_MPLS_*).
I don't think that we should support writing to the MPLS BOS field,
that is, 'writable' should be false for OXM_OF_MPLS_BOS. We don't
support writing to other fields that would essentially result in
packet corruption or require us to reinterpret the flow on the fly
(e.g. we don't support writing to the ethertype or IP protocol).
format_mpls_lse() shows a change in name of mpls_lse_to_stack() to
mpls_lse_to_bos(), implying that this function's name was changed at
some point. It would be better to start out with the preferred name.
In format_mpls_lse_values(), we normally put a space after the
'return' keyword. Also, despite its name, this function does not
format anything (it's a parsing helper, not a formatting helper).
format_odp_action() makes a change in its output format (adding a
comma) that seems should have been incorporated into an earlier patch.
In check_expectations_mpls(), please evaluate check_expectations() in
a statement of its own, because MAX evaluates its arguments more than
once.
(Or, we could write a function that simply returns MAX of two enum
odp_key_fitness parameters. We could call it
greatest_expectations().)
But, looking closer, the only value greater than ODP_FIT_TOO_LITTLE is
ODP_FIT_ERROR, and check_expectations() never returns that, so in fact
check_expectations_mpls() is a constant function that always returns
ODP_FIT_TOO_LITTLE.
Is there a circular calling relationship between parse_l3_onward() and
parse_mpls_onward()? Otherwise the stray prototype for
parse_l3_onward seems a little odd.
There's a change in parse_8021q_onward, in the assignment to
encap_fitness, that I think just changes line breaking without making
any semantic changes.
This change deletes a blank line before commit_set_nw_action() that
it should not.
The checks that this patch adds to ofpact_check__() should happen at
action parsing time instead, because this is not necessary to know the
flow or the maximum number of ports to validate them.
The #if 0...#endif block should not get added to ofpact_format().
There are some typos in the comment on ofpact_push: "MPSL" => "MPLS",
"BPP" => "PBB".
My impression is that 0x8847 is much more common than 0x8848, so
perhaps that should be a default Ethertype for the "push_mpls" action.
The comment in ofputil_usable_protocols() says that OF1.1+ supports
matching on the MPLS stack flag, but in fact this was only added in
OF1.3. (OF1.1 supports the other MPLS matches.)
I left off reading this at packets.c, I'll try to get back to it
later.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] config: Add explicit support for building on ESX.
On Mon, Oct 08, 2012 at 12:49:21PM -0700, Ethan Jackson wrote: > The ESX userspace looks quite a bit like linux, but has some key > differences which need to be specially handled in the build. To > distinguish between ESX and systems use the linux datapath module, > this patch adds two new macros "ESX" and "LINUX_DATAPATH". It uses > these macros to disable building code on ESX which only applies to > a true Linux environment. In addition, it adds a new > route-table-stub implementation which is required for the build to > complete successfully on ESX. > > Signed-off-by: Ethan Jackson Thanks. "-a" isn't portable, so I'd change each of the tests here to test "$HAVE_NETLINK" = yes && test "$ESX" = no > +AM_CONDITIONAL([LINUX_DATAPATH], [test "$HAVE_NETLINK" = yes -a "$ESX" = no]) > +if test "$HAVE_NETLINK" = yes -a "$ESX" = no; then > +AC_DEFINE([LINUX_DATAPATH], [1], [System uses the linux datapath > module.]) > +fi Does the 'argv' code in command-line.c actually malfunction on ESX? It'd be nice to just leave it in, if not. It's not really tied to having the Linux datapath, it's orthogonal. (It should actually work on many Unix and Unix-like systems, but we specialize it to Linux pending testing on other systems.) The new version of the comment in socket-util.c doesn't make sense. I think it should say "if (LINUX_DATAPATH)" instead of "#ifdef __linux__": /* #ifdefs make it a pain to maintain code: you have to try to build both ways. * Thus, this file compiles all of the code regardless of the target, by * writing "if (LINUX)" instead of "#ifdef LINUX_DATAPATH". */ It also changes LINUX so that it's equivalent to LINUX_DATAPATH. That seems odd; perhaps it should instead just do: #ifndef LINUX_DATAPATH #define LINUX_DATAPATH 0 #endif in case that's the intention? Similarly in system-stats.c I see a couple uses of "#if AF_PACKET && LINUX_DATAPATH" in this file. Perhaps adding a configure check for linux/if_packet.h and using HAVE_LINUX_IF_PACKET_H as a test would be better? I think that that actually would better describe the need. Does ESX have /proc/self/fd/* that works like the Linux version? If so then __linux__ is the right test for put_fd_filename(), otherwise LINUX_DATAPATH seems good enough for now. In timeval.c, if we use LINUX_DATAPATH as the check for cache_time, then can we make that a compile-time constant again? Thanks! Ben ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 0/2] datapath: Add flow-based tunneling support
These patches build on the first two patches which Simon Horman sent out in May to move Open vSwitch towards flow-based tunneling. The first patche adds a tun_key, deprecating the tun_id member of the ovs_skb_cb struct. This patche retain compatibilty with existing tunneling, but once the userspace code is submitted, this will be deprecated. The second patch makes an attempt at adding the new tun_key structure into the flow matching logic in the kernel. Please note: A third patch is still needed per Jesse, which is to allow tunnel ports to send and receive packets on ports without destination IP addresses. I'm sending what I have out now to get some feedback, per discussions on the dev list. Kyle Mestery (2): This is a first pass at providing a tun_key which can be used as the basis for flow-based tunnelling. The tun_key includes and replaces the tun_id in both struct ovs_skb_cb and struct sw_tun_key. Move struct ovs_key_ipv4_tunnel out of struct sw_flow_key and into struct sw_flow. This allows it to "float" and be used for matching only when needed. Modify the matching code in ovs_flow_tbl_lookup() to match on the tunnel header if it's set. NEWS| 3 ++ datapath/actions.c | 39 datapath/datapath.c | 43 +- datapath/datapath.h | 6 ++- datapath/flow.c | 104 +++ datapath/flow.h | 27 +++ datapath/tunnel.c | 106 datapath/tunnel.h | 15 ++- datapath/vport-capwap.c | 12 ++--- datapath/vport-gre.c| 15 --- datapath/vport.c| 2 +- include/linux/openvswitch.h | 18 +++- lib/dpif-netdev.c | 1 + lib/odp-util.c | 15 ++- lib/odp-util.h | 3 +- 15 files changed, 303 insertions(+), 106 deletions(-) -- 1.7.11.4 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] datapath: Add support for tun_key to Open vSwitch datapath
This is a first pass at providing a tun_key which can be
used as the basis for flow-based tunnelling. The
tun_key includes and replaces the tun_id in both struct
ovs_skb_cb and struct sw_tun_key.
This patch allows all existing tun_id behaviour to still work. Existing
users of tun_id are redirected to tun_key->tun_id to retain compatibility.
However, when the userspace code is updated to make use of the new tun_key,
the old behaviour will be deprecated and removed.
NOTE: With these changes, the tunneling code no longer assumes input and
output keys are symmetric. If they are not, PMTUD needs to be disabled
for tunneling to work.
Signed-off-by: Kyle Mestery
Cc: Simon Horman
Cc: Jesse Gross
---
V6:
- Fix more comments addressed from Jesse.
V5:
- Address another round of comments from Jesse.
V4:
- Address 2 comments from Jesse:
- When processing actions, if OVS_CB(skb)->tun_key is NULL, point it at one
on the stack temporarily. This goes away when we remove the ability to set
tun_id outside the scope of tun_key.
- Move tun_key to the end of struct sw_flow_key.
V3:
- Fix issues found during review by Jesse.
- Add a NEWS entry around tunnel code no longer assuming symmetric input and
output tunnel keys.
V2:
- Fix blank line addition/removal found by Simon.
- Fix hex printing output found by Simon.
---
NEWS| 3 ++
datapath/actions.c | 38 +
datapath/datapath.c | 10 -
datapath/datapath.h | 5 ++-
datapath/flow.c | 64
datapath/flow.h | 12 --
datapath/tunnel.c | 101
datapath/tunnel.h | 15 ++-
datapath/vport-capwap.c | 12 +++---
datapath/vport-gre.c| 15 ---
datapath/vport.c| 2 +-
include/linux/openvswitch.h | 18 +++-
lib/dpif-netdev.c | 1 +
lib/odp-util.c | 15 ++-
lib/odp-util.h | 3 +-
15 files changed, 235 insertions(+), 79 deletions(-)
diff --git a/NEWS b/NEWS
index 29fd9f3..ae831e3 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@
post-v1.8.0
+- The tunneling code no longer assumes input and output keys are symmetric.
+ If they are not, PMTUD needs to be disabled for tunneling to work. Note
+ this only applies to flow-based keys.
- FreeBSD is now a supported platform, thanks to code contributions from
Gaetano Catalli, Ed Maste, and Giuseppe Lettieri.
- ovs-bugtool: New --ovs option to report only OVS related information.
diff --git a/datapath/actions.c b/datapath/actions.c
index ec9b595..fa8c10d 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -37,7 +37,8 @@
#include "vport.h"
static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
- const struct nlattr *attr, int len, bool keep_skb);
+ const struct nlattr *attr, int len,
+ struct ovs_key_ipv4_tunnel *tun_key, bool keep_skb);
static int make_writable(struct sk_buff *skb, int write_len)
{
@@ -329,11 +330,14 @@ static int sample(struct datapath *dp, struct sk_buff
*skb,
}
return do_execute_actions(dp, skb, nla_data(acts_list),
-nla_len(acts_list), true);
+nla_len(acts_list),
+OVS_CB(skb)->tun_key,
+true);
}
static int execute_set_action(struct sk_buff *skb,
-const struct nlattr *nested_attr)
+const struct nlattr *nested_attr,
+struct ovs_key_ipv4_tunnel *tun_key)
{
int err = 0;
@@ -343,7 +347,21 @@ static int execute_set_action(struct sk_buff *skb,
break;
case OVS_KEY_ATTR_TUN_ID:
- OVS_CB(skb)->tun_id = nla_get_be64(nested_attr);
+ if (OVS_CB(skb)->tun_key == NULL) {
+ /* If tun_key is NULL for this skb, assign it to
+* a value the caller passed in for action processing
+* and output. This can disappear once we drop support
+* for setting tun_id outside of tun_key.
+*/
+ memset(tun_key, 0, sizeof(struct ovs_key_ipv4_tunnel));
+ OVS_CB(skb)->tun_key = tun_key;
+ }
+
+ OVS_CB(skb)->tun_key->tun_id = nla_get_be64(nested_attr);
+ break;
+
+ case OVS_KEY_ATTR_IPV4_TUNNEL:
+ OVS_CB(skb)->tun_key = nla_data(nested_attr);
break;
case OVS_KEY_ATTR_ETHERNET:
@@ -368,7 +386,8 @@ static int execute_set_action(struct sk_buff *skb,
/* Execute a list of actions against 'skb'. */
static int do_ex
[ovs-dev] [PATCH 2/2] datapath: Add flow matching for tunnel keys
Move struct ovs_key_ipv4_tunnel to the top of sw_flow_key. Add a new
struct ovs_flow_hash, which contains both the key_len and the offset to
use when hashing. This allows the outer tunnel to be hashed and used when
looking up flows.
Signed-off-by: Kyle Mestery
---
datapath/actions.c | 1 +
datapath/datapath.c | 33 +-
datapath/datapath.h | 1 +
datapath/flow.c | 68 ++---
datapath/flow.h | 21 +++--
datapath/tunnel.c | 5 ++--
6 files changed, 85 insertions(+), 44 deletions(-)
diff --git a/datapath/actions.c b/datapath/actions.c
index fa8c10d..e430dfc 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -289,6 +289,7 @@ static int output_userspace(struct datapath *dp, struct
sk_buff *skb,
upcall.cmd = OVS_PACKET_CMD_ACTION;
upcall.key = &OVS_CB(skb)->flow->key;
+ upcall.tun_key = &OVS_CB(skb)->flow->key.tun.tun_key;
upcall.userdata = NULL;
upcall.pid = 0;
diff --git a/datapath/datapath.c b/datapath/datapath.c
index d8a198e..b9f95dc 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -313,10 +313,12 @@ void ovs_dp_process_received_packet(struct vport *p,
struct sk_buff *skb)
if (!OVS_CB(skb)->flow) {
struct sw_flow_key key;
+ struct sw_flow_hash flowhash;
int key_len;
/* Extract flow from 'skb' into 'key'. */
- error = ovs_flow_extract(skb, p->port_no, &key, &key_len);
+ error = ovs_flow_extract(skb, p->port_no, &key, &key_len,
+&flowhash);
if (unlikely(error)) {
kfree_skb(skb);
return;
@@ -324,12 +326,13 @@ void ovs_dp_process_received_packet(struct vport *p,
struct sk_buff *skb)
/* Look up flow. */
flow = ovs_flow_tbl_lookup(rcu_dereference(dp->table),
- &key, key_len);
+ &flowhash);
if (unlikely(!flow)) {
struct dp_upcall_info upcall;
upcall.cmd = OVS_PACKET_CMD_MISS;
upcall.key = &key;
+ upcall.tun_key = &key.tun.tun_key;
upcall.userdata = NULL;
upcall.pid = p->upcall_pid;
ovs_dp_upcall(dp, skb, &upcall);
@@ -747,6 +750,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb,
struct genl_info *info)
struct ovs_header *ovs_header = info->userhdr;
struct nlattr **a = info->attrs;
struct sw_flow_actions *acts;
+ struct sw_flow_hash flowhash;
struct sk_buff *packet;
struct sw_flow *flow;
struct datapath *dp;
@@ -787,7 +791,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb,
struct genl_info *info)
if (IS_ERR(flow))
goto err_kfree_skb;
- err = ovs_flow_extract(packet, -1, &flow->key, &key_len);
+ err = ovs_flow_extract(packet, -1, &flow->key, &key_len, &flowhash);
if (err)
goto err_flow_put;
@@ -802,7 +806,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb,
struct genl_info *info)
if (err)
goto err_flow_put;
- flow->hash = ovs_flow_hash(&flow->key, key_len);
+ flow->hash = ovs_flow_hash(&flowhash);
acts = ovs_flow_actions_alloc(a[OVS_PACKET_ATTR_ACTIONS]);
err = PTR_ERR(acts);
@@ -1017,6 +1021,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
struct ovs_header *ovs_header = info->userhdr;
struct sw_flow_key key;
struct sw_flow *flow;
+ struct sw_flow_hash flowhash;
struct sk_buff *reply;
struct datapath *dp;
struct flow_table *table;
@@ -1027,7 +1032,8 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
error = -EINVAL;
if (!a[OVS_FLOW_ATTR_KEY])
goto error;
- error = ovs_flow_from_nlattrs(&key, &key_len, a[OVS_FLOW_ATTR_KEY]);
+ error = ovs_flow_from_nlattrs(&key, &key_len, &flowhash,
+ a[OVS_FLOW_ATTR_KEY]);
if (error)
goto error;
@@ -1047,7 +1053,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
goto error;
table = genl_dereference(dp->table);
- flow = ovs_flow_tbl_lookup(table, &key, key_len);
+ flow = ovs_flow_tbl_lookup(table, &flowhash);
if (!flow) {
struct sw_flow_actions *acts;
@@ -1085,7 +1091,8 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
rcu_assign_pointer(flow->sf_acts, acts);
/* Put flow in bucket. */
- flow->hash = ovs_flow_hash(&key, ke
[ovs-dev] [PATCH] datapath: Fix GRE tunnel without any key.
Commit 2de795adb96 (datapath: 64-bit GRE support) introduced a bug
for tunnels with no key. Following patch fixes it by setting tunnel
type to GRE type.
Bugs: 13511
Signed-off-by: Pravin B Shelar
---
datapath/vport-gre.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
index 6f406bc..4000c74 100644
--- a/datapath/vport-gre.c
+++ b/datapath/vport-gre.c
@@ -202,8 +202,11 @@ static int parse_header(struct iphdr *iph, __be16 *flags,
__be64 *tun_id,
*tunnel_type = TNL_T_PROTO_GRE;
}
*tun_id = key_to_tunnel_id(gre_key, seq);
- } else
+ } else {
*tun_id = 0;
+ /* Ignore GRE seq if there is no key present. */
+ *tunnel_type = TNL_T_PROTO_GRE;
+ }
if (greh->flags & GRE_SEQ)
hdr_len += GRE_HEADER_SECTION;
--
1.7.10
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Fix GRE tunnel without any key.
On Oct 9, 2012, at 2:55 PM, Pravin B Shelar wrote: > Commit 2de795adb96 (datapath: 64-bit GRE support) introduced a bug > for tunnels with no key. Following patch fixes it by setting tunnel > type to GRE type. > > Bugs: 13511 > Signed-off-by: Pravin B Shelar Acked-by: Kyle Mestery ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] config: Add explicit support for building on ESX.
> Does the 'argv' code in command-line.c actually malfunction on ESX? > It'd be nice to just leave it in, if not. It's not really tied to > having the Linux datapath, it's orthogonal. (It should actually work > on many Unix and Unix-like systems, but we specialize it to Linux > pending testing on other systems.) Yes I disabled it because I had verified that according to ps the process title was not set. > Does ESX have /proc/self/fd/* that works like the Linux version? If > so then __linux__ is the right test for put_fd_filename(), otherwise > LINUX_DATAPATH seems good enough for now. Nope, it doesn't have /proc/self/fd, or /proc/self for that matter. > In timeval.c, if we use LINUX_DATAPATH as the check for cache_time, > then can we make that a compile-time constant again? I'm not sure I totally understand what you're asking for here. You want cache_time to be a constant? That can be changed at runtime in some cases. I'll send out an incremental shortly. Ethan > > Thanks! > > Ben ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] config: Add explicit support for building on ESX.
On Tue, Oct 09, 2012 at 01:54:28PM -0700, Ethan Jackson wrote: > > Does the 'argv' code in command-line.c actually malfunction on ESX? > > It'd be nice to just leave it in, if not. It's not really tied to > > having the Linux datapath, it's orthogonal. (It should actually work > > on many Unix and Unix-like systems, but we specialize it to Linux > > pending testing on other systems.) > > Yes I disabled it because I had verified that according to ps the > process title was not set. OK, thanks. > > Does ESX have /proc/self/fd/* that works like the Linux version? If > > so then __linux__ is the right test for put_fd_filename(), otherwise > > LINUX_DATAPATH seems good enough for now. > > Nope, it doesn't have /proc/self/fd, or /proc/self for that matter. OK, thanks. > > In timeval.c, if we use LINUX_DATAPATH as the check for cache_time, > > then can we make that a compile-time constant again? > > I'm not sure I totally understand what you're asking for here. You > want cache_time to be a constant? That can be changed at runtime in > some cases. Maybe I'm mistaken, but IIRC ESX is the only case we know of where timer_create() doesn't work. We previously found this out only at runtime, but with your change we will instead disable timer caching at compile time so there's now (again) no need to disable it at runtime. No? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] config: Add explicit support for building on ESX.
> Maybe I'm mistaken, but IIRC ESX is the only case we know of where > timer_create() doesn't work. We previously found this out only at > runtime, but with your change we will instead disable timer caching at > compile time so there's now (again) no need to disable it at runtime. > No? True, I am expecting that in future versions of ESX, timer_create will be implemented, so I wanted to leave it like this so that we will do the right thing in both cases. That said, we can always make it a run time check if/when we need it. I'll change it back for now. Ethan ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] config: Add explicit support for building on ESX.
On Tue, Oct 09, 2012 at 02:21:08PM -0700, Ethan Jackson wrote: > > Maybe I'm mistaken, but IIRC ESX is the only case we know of where > > timer_create() doesn't work. We previously found this out only at > > runtime, but with your change we will instead disable timer caching at > > compile time so there's now (again) no need to disable it at runtime. > > No? > > True, I am expecting that in future versions of ESX, timer_create will > be implemented, so I wanted to leave it like this so that we will do > the right thing in both cases. That said, we can always make it a run > time check if/when we need it. I'll change it back for now. Thanks. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Changing proc title on FreeBSD
On 9 October 2012 17:18, Ben Pfaff wrote: > On Tue, Oct 09, 2012 at 01:54:28PM -0700, Ethan Jackson wrote: >> > Does the 'argv' code in command-line.c actually malfunction on ESX? >> > It'd be nice to just leave it in, if not. It's not really tied to >> > having the Linux datapath, it's orthogonal. (It should actually work >> > on many Unix and Unix-like systems, but we specialize it to Linux >> > pending testing on other systems.) >> >> Yes I disabled it because I had verified that according to ps the >> process title was not set. > > OK, thanks. Interesting - I didn't notice this code before. On FreeBSD we have setproctitle(const char *fmt, ...) for this functionality, which has the same interface as proctitle_set. Unfortunately there's no decent way to pass the varargs through. I could add an #elif defined(__FreeBSD__) case to command-line.c and use a temporary buffer in my proctitle_set implementation, or could just #define proctitle_set setproctitle in command-line.h. Thoughts on which is preferable? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Changing proc title on FreeBSD
On Tue, Oct 09, 2012 at 06:03:32PM -0400, Ed Maste wrote: > On 9 October 2012 17:18, Ben Pfaff wrote: > > On Tue, Oct 09, 2012 at 01:54:28PM -0700, Ethan Jackson wrote: > >> > Does the 'argv' code in command-line.c actually malfunction on ESX? > >> > It'd be nice to just leave it in, if not. It's not really tied to > >> > having the Linux datapath, it's orthogonal. (It should actually work > >> > on many Unix and Unix-like systems, but we specialize it to Linux > >> > pending testing on other systems.) > >> > >> Yes I disabled it because I had verified that according to ps the > >> process title was not set. > > > > OK, thanks. > > Interesting - I didn't notice this code before. > > On FreeBSD we have setproctitle(const char *fmt, ...) for this > functionality, which has the same interface as proctitle_set. > Unfortunately there's no decent way to pass the varargs through. > > I could add an #elif defined(__FreeBSD__) case to command-line.c and > use a temporary buffer in my proctitle_set implementation, or could > just #define proctitle_set setproctitle in command-line.h. Thoughts > on which is preferable? I'm happy enough with the latter; it's simpler, as long as it works. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] config: Add explicit support for building on ESX.
Here's an incremental.
---
configure.ac| 4 ++--
lib/socket-util.c | 12 +---
lib/timeval.c | 42 +++---
lib/timeval.h | 16
tests/test-timeval.c| 4 ++--
vswitchd/system-stats.c | 21 ++---
6 files changed, 42 insertions(+), 57 deletions(-)
diff --git a/configure.ac b/configure.ac
index ba1e936..6fb30fd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -113,8 +113,8 @@ AC_CONFIG_COMMANDS([include/openflow/openflow.h.stamp])
AC_CONFIG_COMMANDS([ovsdb/ovsdbmonitor/dummy], [:])
AC_CONFIG_COMMANDS([utilities/bugtool/dummy], [:])
-AM_CONDITIONAL([LINUX_DATAPATH], [test "$HAVE_NETLINK" = yes -a "$ESX" = no])
-if test "$HAVE_NETLINK" = yes -a "$ESX" = no; then
+AM_CONDITIONAL([LINUX_DATAPATH], [test "$HAVE_NETLINK" = yes && test "$ESX" =
no])
+if test "$HAVE_NETLINK" = yes && test "$ESX" = no; then
AC_DEFINE([LINUX_DATAPATH], [1], [System uses the linux datapath module.])
fi
diff --git a/lib/socket-util.c b/lib/socket-util.c
index ca9e406..a37dfe4 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -51,11 +51,9 @@ VLOG_DEFINE_THIS_MODULE(socket_util);
/* #ifdefs make it a pain to maintain code: you have to try to build both ways.
* Thus, this file compiles all of the code regardless of the target, by
- * writing "if (LINUX)" instead of "#ifdef LINUX_DATAPATH". */
-#ifdef LINUX_DATAPATH
-#define LINUX 1
-#else
-#define LINUX 0
+ * writing "if (LINUX_DATAPATH)" instead of "#ifdef __linux__". */
+#ifndef LINUX_DATAPATH
+#define LINUX_DATAPATH 0
#endif
#ifndef O_DIRECTORY
@@ -266,7 +264,7 @@ drain_rcvbuf(int fd)
*
* On other Unix-like OSes, MSG_TRUNC has no effect in the flags
* argument. */
-char buffer[LINUX ? 1 : 2048];
+char buffer[LINUX_DATAPATH ? 1 : 2048];
ssize_t n_bytes = recv(fd, buffer, sizeof buffer,
MSG_TRUNC | MSG_DONTWAIT);
if (n_bytes <= 0 || n_bytes >= rcvbuf) {
@@ -335,7 +333,7 @@ make_sockaddr_un(const char *name, struct sockaddr_un *un,
socklen_t *un_len,
if (strlen(name) > MAX_UN_LEN) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-if (LINUX) {
+if (LINUX_DATAPATH) {
/* 'name' is too long to fit in a sockaddr_un, but we have a
* workaround for that on Linux: shorten it by opening a file
* descriptor for the directory part of the name and indirecting
diff --git a/lib/timeval.c b/lib/timeval.c
index 9c9cf0e..d3b6685 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -40,15 +40,7 @@ VLOG_DEFINE_THIS_MODULE(timeval);
* to CLOCK_REALTIME. */
static clockid_t monotonic_clock;
-/* Controls whether or not calls to clock_gettime() are cached. See
- * time_cached() for a detailed explanation. */
-#if defined __x86_64__ && defined LINUX_DATAPATH
-static bool cache_time = false;
-#else
-static bool cache_time = true;
-#endif
-
-/* Has a timer tick occurred? Only relevant if cache_time is true.
+/* Has a timer tick occurred? Only relevant if CACHE_TIME is true.
*
* We initialize these to true to force time_init() to get called on the first
* call to time_msec() or another function that queries the current time. */
@@ -154,15 +146,12 @@ set_up_timer(void)
static timer_t timer_id;/* "static" to avoid apparent memory leak. */
struct itimerspec itimer;
-if (!cache_time) {
+if (!CACHE_TIME) {
return;
}
if (timer_create(monotonic_clock, NULL, &timer_id)) {
-VLOG_WARN("timer_create failed (%s), disabling cached timing",
- strerror(errno));
-cache_time = false;
-return;
+VLOG_FATAL("timer_create failed (%s)", strerror(errno));
}
itimer.it_interval.tv_sec = 0;
@@ -215,7 +204,7 @@ refresh_monotonic(void)
/* Forces a refresh of the current time from the kernel. It is not usually
* necessary to call this function, since the time will be refreshed
* automatically at least every TIME_UPDATE_INTERVAL milliseconds. If
- * cache_time is false, we will always refresh the current time so this
+ * CACHE_TIME is false, we will always refresh the current time so this
* function has no effect. */
void
time_refresh(void)
@@ -356,7 +345,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long
int timeout_when,
break;
}
-if (!blocked && !cache_time) {
+if (!blocked && !CACHE_TIME) {
block_sigalrm(&oldsigs);
blocked = true;
}
@@ -370,23 +359,6 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long
int timeout_when,
return retval;
}
-/* True on systems (particularly x86-64 Linux) where clock_gettime() is
- * inexpensive. On these systems, we don't bother caching the current time.
- * Instead, we consult clock_gettime() directly when needed.
- *
- * False on systems where clock_get
Re: [ovs-dev] [PATCH] config: Add explicit support for building on ESX.
On Tue, Oct 09, 2012 at 03:09:51PM -0700, Ethan Jackson wrote: > Here's an incremental. Thanks for humoring me, this seems fine. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] config: Add explicit support for building on ESX.
Thanks for the review, I'll merge this shortly. Ethan On Tue, Oct 9, 2012 at 3:16 PM, Ben Pfaff wrote: > On Tue, Oct 09, 2012 at 03:09:51PM -0700, Ethan Jackson wrote: >> Here's an incremental. > > Thanks for humoring me, this seems fine. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] Set de cuchillos con hoja de cerámica / Curso English Support / Visita Jérez de la Frontera
Cabestan. Layout Simple Asegúrate de no perderte ninguna oferta, añade ofer...@globalbono.com a tu lista de contactos. Si no ves correctamente las imágenes, pulsa [ http://email.globalbono.com/E09102012154627.cfm?WL=1845&WS=208833_8707685&WA=965 ] aquí [ http://email.globalbono.com/Go/index.cfm?WL=564&WS=208833_8707685&WA=965 ] Tus Ofertas de hoy Síguenos en: [ http://email.globalbono.com/Go/index.cfm?WL=50&WS=208833_8707685&WA=965 ] [ http://email.globalbono.com/Go/index.cfm?WL=51&WS=208833_8707685&WA=965 ] [ http://email.globalbono.com/Go/index.cfm?WL=1152&WS=208833_8707685&WA=965 ] [ http://email.globalbono.com/Go/index.cfm?WL=1732&WS=208833_8707685&WA=965 ] 52% Dto. Máquina de Coser Portátil. El modista en casa. Se acabó el tener que pagar por esos arreglos textiles que tú sabes hacer perfectamente. Hasta ahora te... 55,62¤ [ http://email.globalbono.com/Go/index.cfm?WL=1732&WS=208833_8707685&WA=965 ] Antes 116,00¤Dto: 52% Ahorro: 60,38¤ [ http://email.globalbono.com/Go/index.cfm?WL=472&WS=208833_8707685&WA=965 ] 73% Dto. Cigarrillo Electrónico Si deseas dejar de fumar, éste es el aliado adecuado. Calmará esa costumbre de ponerte algo en la bo... 10,49¤ [ http://email.globalbono.com/Go/index.cfm?WL=472&WS=208833_8707685&WA=965 ] Antes 39,00¤Dto: 73% Ahorro: 28,51¤ [ http://email.globalbono.com/Go/index.cfm?WL=1830&WS=208833_8707685&WA=965 ] 52% Dto. Disfruta visitando Jerez de la Frontera y cenando en El Bocado . San Valentín aún queda muy lejos y todos sabemos que las relaciones hay que cultivarlas día a día. Por ... 56,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=1830&WS=208833_8707685&WA=965 ] Antes 116,00¤Dto: 52% Ahorro: 60,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=1736&WS=208833_8707685&WA=965 ] 56% Dto. Cuchillos con hoja de cerámica. Corta con estilo. Si eres un amante de la buena cocina y no solo te agrada degustarla sino que además eres un experto coc... 22,24¤ [ http://email.globalbono.com/Go/index.cfm?WL=1736&WS=208833_8707685&WA=965 ] Antes 51,00¤Dto: 56% Ahorro: 28,76¤ [ http://email.globalbono.com/Go/index.cfm?WL=1735&WS=208833_8707685&WA=965 ] 59% Dto. Goza de una experiencia salvaje en Senda Viva Sumérgete en plena naturaleza con esta oferta que te presentamos para visitar el parque Senda Viva. Pod... 64,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=1735&WS=208833_8707685&WA=965 ] Antes 156,00¤Dto: 59% Ahorro: 92,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=1831&WS=208833_8707685&WA=965 ] 84% Dto. Curso English Support FIRST online Con este curso online conseguirás establecer fácilmente una dinámica diaria de estudio gracias a la cua... 19,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=1831&WS=208833_8707685&WA=965 ] Antes 120,00¤Dto: 84% Ahorro: 101,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=1737&WS=208833_8707685&WA=965 ] 61% Dto. Una gran noche romántica te espera en el HOTEL SPA TORREPACHECO Entre la gran variedad de ofertas que te presentan centros de Spa a buen precio debes elegir ante todo ... 84,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=1737&WS=208833_8707685&WA=965 ] Antes 215,00¤Dto: 61% Ahorro: 131,00¤ [ http://email.globalbono.com/Go/index.cfm?WL=706&WS=208833_8707685&WA=965 ] De conformidad con lo que establece la Ley Orgánica 15/1999 de Protección de Datos de Carácter Personal, le informamos que sus datos personales serán incorporados a un fichero bajo la responsabilidad de GlobalBono, con la finalidad de poder atender los compromisos derivados de la relación que mantenemos con usted. Puede ejercer sus derechos de acceso, cancelación, rectificación y oposición mediante un escrito a la dirección email i...@globalbono.com. Si en el plazo de 30 días no nos comunica lo contrario, entenderemos que los datos no han sido modificados, que se compromete a notificarnos cualquier variación y que tenemos el consentimiento para utilizarlos a fin de poder fidelizar la relación entre las partes. ¿Quieres dejar de recibir este email? Haz click [ http://email.globalbono.com/baja/form_baja_GB.cfm?WL=182&WS=208833_8707685&WA=965 ] aquí. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Fix GRE tunnel without any key.
On Tue, Oct 9, 2012 at 12:57 PM, Kyle Mestery (kmestery) wrote: > On Oct 9, 2012, at 2:55 PM, Pravin B Shelar wrote: >> Commit 2de795adb96 (datapath: 64-bit GRE support) introduced a bug >> for tunnels with no key. Following patch fixes it by setting tunnel >> type to GRE type. >> >> Bugs: 13511 >> Signed-off-by: Pravin B Shelar > > > Acked-by: Kyle Mestery Thanks, Pushed to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] datapath: Allow GRE64 tunnels without any key.
Now GRE64 deals with tunnel with no key and tunnel with zero key
as same. This behaviour is different than standard GRE.
Signed-off-by: Pravin B Shelar
---
datapath/vport-gre.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
index 4000c74..e3a190f 100644
--- a/datapath/vport-gre.c
+++ b/datapath/vport-gre.c
@@ -54,7 +54,11 @@ static int gre_hdr_len(const struct tnl_mutable_config
*mutable)
if (mutable->flags & TNL_F_CSUM)
len += GRE_HEADER_SECTION;
- if (mutable->out_key || mutable->flags & TNL_F_OUT_KEY_ACTION) {
+ /* Set key for GRE64 tunnels, even when key if is zero. */
+ if (mutable->out_key ||
+ mutable->key.tunnel_type & TNL_T_PROTO_GRE64 ||
+ mutable->flags & TNL_F_OUT_KEY_ACTION) {
+
len += GRE_HEADER_SECTION;
if (mutable->key.tunnel_type & TNL_T_PROTO_GRE64)
len += GRE_HEADER_SECTION;
@@ -103,7 +107,8 @@ static void gre_build_header(const struct vport *vport,
if (mutable->key.tunnel_type & TNL_T_PROTO_GRE64)
greh->flags |= GRE_SEQ;
- } else if (mutable->out_key) {
+ } else if (mutable->out_key ||
+ mutable->key.tunnel_type & TNL_T_PROTO_GRE64) {
greh->flags |= GRE_KEY;
*options = be64_get_low32(mutable->out_key);
if (mutable->key.tunnel_type & TNL_T_PROTO_GRE64) {
@@ -131,7 +136,8 @@ static struct sk_buff *gre_update_header(const struct vport
*vport,
}
*options = be64_get_low32(OVS_CB(skb)->tun_id);
options--;
- } else if (mutable->out_key) {
+ } else if (mutable->out_key ||
+ mutable->key.tunnel_type & TNL_T_PROTO_GRE64) {
options--;
if (mutable->key.tunnel_type & TNL_T_PROTO_GRE64)
options--;
--
1.7.10
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Allow GRE64 tunnels without any key.
On Tue, Oct 9, 2012 at 4:31 PM, Pravin B Shelar wrote: > Now GRE64 deals with tunnel with no key and tunnel with zero key > as same. This behaviour is different than standard GRE. > > Signed-off-by: Pravin B Shelar Acked-by: Jesse Gross ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath: Allow GRE64 tunnels without any key.
On Tue, Oct 9, 2012 at 5:10 PM, Jesse Gross wrote: > On Tue, Oct 9, 2012 at 4:31 PM, Pravin B Shelar wrote: >> Now GRE64 deals with tunnel with no key and tunnel with zero key >> as same. This behaviour is different than standard GRE. >> >> Signed-off-by: Pravin B Shelar > > Acked-by: Jesse Gross Thanks, Pushed to master. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/4] datapath: Add basic MPLS support to kernel
On Tue, Oct 09, 2012 at 04:08:32PM +0900, Simon Horman wrote:
> +static int push_mpls(struct sk_buff *skb, const struct ovs_action_push_mpls
> *mpls)
> +{
> + u32 l2_size;
> + __be32 *new_mpls_label;
> +
> + if (skb_cow_head(skb, MPLS_HLEN) < 0) {
> + kfree_skb(skb);
> + return -ENOMEM;
> + }
> +
> + l2_size = skb_cb_mpls_stack_offset(skb);
> + skb_push(skb, MPLS_HLEN);
> + memmove(skb_mac_header(skb) - MPLS_HLEN, skb_mac_header(skb), l2_size);
> + skb_reset_mac_header(skb);
> +
> + new_mpls_label = (__be32 *)(skb_mac_header(skb) + l2_size);
> + *new_mpls_label = mpls->mpls_label;
> +
> + set_ethertype(skb, mpls->mpls_ethertype);
> + return 0;
> +}
> +
> +static int pop_mpls(struct sk_buff *skb, const __be16 *ethertype)
> +{
> + __be16 current_ethertype = get_ethertype(skb);
> + if (current_ethertype == htons(ETH_P_MPLS_UC) ||
> + current_ethertype == htons(ETH_P_MPLS_MC)) {
> + u32 l2_size = skb_cb_mpls_stack_offset(skb);
> +
> + memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
> l2_size);
> +
> + skb_pull(skb, MPLS_HLEN);
> + skb_reset_mac_header(skb);
> +
> + set_ethertype(skb, *ethertype);
> + }
> + return 0;
> +}
> +
> +static int set_mpls(struct sk_buff *skb, const __be32 *mpls_label)
> +{
> + __be16 current_ethertype = get_ethertype(skb);
> + if (current_ethertype == htons(ETH_P_MPLS_UC) ||
> + current_ethertype == htons(ETH_P_MPLS_MC)) {
> + memcpy(skb_cb_mpls_stack(skb), mpls_label, sizeof(__be32));
> + }
> + return 0;
> +}
> +
push_mpls and set_mpls overwrite BOS bit. So it can produce corrupted packet.
Is this by design?
--
yamahata
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/4] user-space datapath: Add basic MPLS support to kernel
On Tue, Oct 09, 2012 at 04:08:33PM +0900, Simon Horman wrote:
> This is to match the kernel implementation in the
> patch "datapath: Add basic MPLS support to kernel"
> by Leo Alterman.
>
> Cc: Leo Alterman
> Signed-off-by: Simon Horman
>
> ---
>
> v2.2
> * No change
>
> v2.1
> * Initial post
> ---
> lib/dpif-netdev.c | 13 +
> lib/odp-util.c| 43 +++
> 2 files changed, 56 insertions(+)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index c9e3210..6deb9ff 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1212,6 +1212,7 @@ execute_set_action(struct ofpbuf *packet, const struct
> nlattr *a)
> case OVS_KEY_ATTR_ICMPV6:
> case OVS_KEY_ATTR_ARP:
> case OVS_KEY_ATTR_ND:
> + case OVS_KEY_ATTR_MPLS:
> case __OVS_KEY_ATTR_MAX:
> default:
> NOT_REACHED();
> @@ -1249,6 +1250,18 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
> eth_pop_vlan(packet);
> break;
>
> +case OVS_ACTION_ATTR_PUSH_MPLS:
> +push_mpls(packet, nl_attr_get_be16(a));
> +break;
> +
> +case OVS_ACTION_ATTR_POP_MPLS:
> + pop_mpls(packet, nl_attr_get_be16(a));
> + break;
> +
> +case OVS_ACTION_ATTR_SET_MPLS:
> + set_mpls_lse(packet, nl_attr_get_be32(a));
> + break;
> +
> case OVS_ACTION_ATTR_SET:
> execute_set_action(packet, nl_attr_get(a));
> break;
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 257d7a7..13b3248 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -73,6 +73,9 @@ odp_action_len(uint16_t type)
> case OVS_ACTION_ATTR_USERSPACE: return -2;
> case OVS_ACTION_ATTR_PUSH_VLAN: return sizeof(struct
> ovs_action_push_vlan);
> case OVS_ACTION_ATTR_POP_VLAN: return 0;
> +case OVS_ACTION_ATTR_PUSH_MPLS: return sizeof(struct
> ovs_action_push_mpls);
> +case OVS_ACTION_ATTR_POP_MPLS: return sizeof(ovs_be16);
> +case OVS_ACTION_ATTR_SET_MPLS: return sizeof(ovs_be32);
> case OVS_ACTION_ATTR_SET: return -2;
> case OVS_ACTION_ATTR_SAMPLE: return -2;
>
> @@ -106,6 +109,7 @@ ovs_key_attr_to_string(enum ovs_key_attr attr)
> case OVS_KEY_ATTR_ARP: return "arp";
> case OVS_KEY_ATTR_ND: return "nd";
> case OVS_KEY_ATTR_TUN_ID: return "tun_id";
> +case OVS_KEY_ATTR_MPLS: return "mpls";
>
> case __OVS_KEY_ATTR_MAX:
> default:
> @@ -273,6 +277,17 @@ format_vlan_tci(struct ds *ds, ovs_be16 vlan_tci)
> }
>
> static void
> +format_mpls_lse(struct ds *ds, ovs_be32 mpls_lse)
> +{
> +ds_put_format(ds, "label=%"PRIu32",tc=%d,ttl=%d,bos=%d",
> + mpls_lse_to_label(mpls_lse),
> + mpls_lse_to_tc(mpls_lse),
> + mpls_lse_to_ttl(mpls_lse),
> + mpls_lse_to_stack(mpls_lse));
> +}
> +
> +
> +static void
> format_odp_action(struct ds *ds, const struct nlattr *a)
> {
> int expected_len;
> @@ -311,6 +326,25 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
> case OVS_ACTION_ATTR_POP_VLAN:
> ds_put_cstr(ds, "pop_vlan");
> break;
> +case OVS_ACTION_ATTR_PUSH_MPLS: {
> +const struct ovs_action_push_mpls *mpls = nl_attr_get(a);
> +ds_put_cstr(ds, "push_mpls(");
> +format_mpls_lse(ds, mpls->mpls_label);
> +ds_put_format(ds, "eth_type=0x%"PRIx16")",
> ntohs(mpls->mpls_ethertype));
> +break;
> +}
> +case OVS_ACTION_ATTR_POP_MPLS: {
> +ovs_be16 ethertype = nl_attr_get_be16(a);
> +ds_put_format(ds, "pop_mpls(eth_type=0x%"PRIx16")",
> ntohs(ethertype));
> +break;
> + }
Minor nitpick. the position of '}'
thanks,
> +case OVS_ACTION_ATTR_SET_MPLS: {
> +ovs_be32 label = nl_attr_get_be32(a);
> +ds_put_cstr(ds, "set_mpls(");
> +format_mpls_lse(ds, label);
> +ds_put_char(ds, ')');
> +break;
> +}
> case OVS_ACTION_ATTR_SAMPLE:
> format_odp_sample_action(ds, a);
> break;
> @@ -606,6 +640,7 @@ odp_flow_key_attr_len(uint16_t type)
> case OVS_KEY_ATTR_ETHERNET: return sizeof(struct ovs_key_ethernet);
> case OVS_KEY_ATTR_VLAN: return sizeof(ovs_be16);
> case OVS_KEY_ATTR_ETHERTYPE: return 2;
> +case OVS_KEY_ATTR_MPLS: return sizeof(struct ovs_key_mpls);
> case OVS_KEY_ATTR_IPV4: return sizeof(struct ovs_key_ipv4);
> case OVS_KEY_ATTR_IPV6: return sizeof(struct ovs_key_ipv6);
> case OVS_KEY_ATTR_TCP: return sizeof(struct ovs_key_tcp);
> @@ -715,6 +750,14 @@ format_odp_key_attr(const struct nlattr *a, struct ds
> *ds)
> ds_put_char(ds, ')');
> break;
>
> +case OVS_KEY_ATTR_MPLS: {
> +const struct ovs_key_mpls *mpls_key = nl_attr_get(a);
> +ds_put_char(ds, '(');
> +format_mpls_lse(ds, mpls_key->mpls_top_label);
> +ds_put_char(ds, ')');
> +
Re: [ovs-dev] [PATCH 4/4] User-Space MPLS actions and matches
On Tue, Oct 09, 2012 at 04:08:35PM +0900, Simon Horman wrote: > @@ -938,6 +990,15 @@ mf_is_value_valid(const struct mf_field *mf, const union > mf_value *value) > case MFF_IPV6_LABEL: > return !(value->be32 & ~htonl(IPV6_LABEL_MASK)); > > +case MFF_MPLS_LABEL: > +return !(value->be32 & ~htonl(MPLS_LABEL_MASK >> MPLS_LABEL_SHIFT)); > + > +case MFF_MPLS_TC: > +return !(value->u8 & ~7); > + > +case MFF_MPLS_BOS: > +return !(value->u8 & ~1); > + Minor nitpick. MPLS_xxx_MASK >> MPLS_xxx_SHIFT. > @@ -1794,6 +1906,18 @@ mf_random_value(const struct mf_field *mf, union > mf_value *value) > value->u8 &= 0x07; > break; > > +case MFF_MPLS_LABEL: > +value->be32 &= htonl(MPLS_LABEL_MASK >> MPLS_LABEL_SHIFT); > +break; > + > +case MFF_MPLS_TC: > +value->u8 &= 0x07; > +break; > + > +case MFF_MPLS_BOS: > +value->u8 &= 0x01; > +break; > + ditto. -- yamahata ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
