Re: [ovs-dev] [PATCH 1/3] ofproto-dpif-sflow: propagate actions within clone

2017-12-12 Thread Gregory Rose 

On 12/5/2017 2:43 AM, Zoltan Balogh wrote:

By avoiding Tx recirculation and embracing tnl_push action within clone,
the tunnel metadata is not propagated. Unless clone action is handled in
the dpif_sflow_read_actions() function as well. This commit resolves
this issue.
In addition, some sflow data needs to be stored and restored in
ofproto-dpif-xlate when native_tunnel_output() is invoked. Otherwise the
output action of underlay bridge is getting counted as well if sFlow is
set on the overlay bridge.

Signed-off-by: Zoltan Balogh 
CC: Sugesh Chandran 
---
  ofproto/ofproto-dpif-sflow.c  | 19 +--
  ofproto/ofproto-dpif-sflow.h  |  2 +-
  ofproto/ofproto-dpif-upcall.c |  2 +-
  ofproto/ofproto-dpif-xlate.c  |  7 +++
  tests/ofproto-dpif.at |  2 +-
  5 files changed, 23 insertions(+), 9 deletions(-)


There are checkpatch errors on this patch.

== Checking 
"0001-ofproto-dpif-sflow-propagate-actions-within-clone.patch" ==

WARNING: Line has non-spaces leading whitespace
#38 FILE: ofproto/ofproto-dpif-sflow.c:1092:
    struct dpif_sflow_actions *sflow_actions, bool 
capture_mpls)


WARNING: Line length is >79-characters long
WARNING: Line has non-spaces leading whitespace
#84 FILE: ofproto/ofproto-dpif-sflow.c:1347:
    && (num_outputs == 1 || num_outputs == 2)) { /* push_tnl or 
clone + push_tnl */


WARNING: Line has non-spaces leading whitespace
#93 FILE: ofproto/ofproto-dpif-sflow.c:1377:
    && num_outputs == 1) {

WARNING: Line has non-spaces leading whitespace
#106 FILE: ofproto/ofproto-dpif-sflow.h:74:
 struct dpif_sflow_actions *, bool 
capture_mpls);


Lines checked: 164, Warnings: 5, Errors: 0

- Greg


diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index ccf89645b..0bdb3fdfa 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -961,7 +961,7 @@ sflow_read_set_action(const struct nlattr *attr,
  } else {
  dpif_sflow_read_actions(NULL,
  nl_attr_get(attr), nl_attr_get_size(attr),
-sflow_actions);
+sflow_actions, true);
  }
  break;
  case OVS_KEY_ATTR_PRIORITY:
@@ -1089,7 +1089,7 @@ dpif_sflow_capture_input_mpls(const struct flow *flow,
  void
  dpif_sflow_read_actions(const struct flow *flow,
const struct nlattr *actions, size_t actions_len,
-   struct dpif_sflow_actions *sflow_actions)
+   struct dpif_sflow_actions *sflow_actions, bool 
capture_mpls)
  {
  const struct nlattr *a;
  unsigned int left;
@@ -1099,7 +1099,7 @@ dpif_sflow_read_actions(const struct flow *flow,
return;
  }
  
-if (flow != NULL) {

+if (flow != NULL && capture_mpls == true) {
/* Make sure the MPLS output stack
 * is seeded with the input stack.
 */
@@ -1197,8 +1197,13 @@ dpif_sflow_read_actions(const struct flow *flow,
 * structure to report this.
 */
break;
+case OVS_ACTION_ATTR_CLONE:
+if (flow != NULL) {
+dpif_sflow_read_actions(flow, nl_attr_get(a), nl_attr_get_size(a),
+sflow_actions, false);
+}
+break;
case OVS_ACTION_ATTR_SAMPLE:
-   case OVS_ACTION_ATTR_CLONE:
  case OVS_ACTION_ATTR_ENCAP_NSH:
  case OVS_ACTION_ATTR_DECAP_NSH:
case OVS_ACTION_ATTR_UNSPEC:
@@ -1266,6 +1271,7 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
dp_packet *packet,
  struct dpif_sflow_port *in_dsp;
  struct dpif_sflow_port *out_dsp;
  ovs_be16 vlan_tci;
+uint32_t num_outputs;
  
  ovs_mutex_lock();

  sampler = ds->sflow_agent->samplers;
@@ -1333,11 +1339,12 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
dp_packet *packet,
}
  }
  
+num_outputs = dpif_sflow_cookie_num_outputs(cookie);

  /* Output tunnel. */
  if (sflow_actions
&& sflow_actions->encap_depth == 1
&& !sflow_actions->tunnel_err
-   && dpif_sflow_cookie_num_outputs(cookie) == 1) {
+   && (num_outputs == 1 || num_outputs == 2)) { /* push_tnl or clone + 
push_tnl */
tnlOutProto = sflow_actions->tunnel_ipproto;
if (tnlOutProto == 0) {
/* Try to infer the ip-protocol from the output port. */
@@ -1367,7 +1374,7 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
dp_packet *packet,
  if (sflow_actions
&& sflow_actions->mpls_stack_depth > 0
&& !sflow_actions->mpls_err
-   && dpif_sflow_cookie_num_outputs(cookie) == 1) {
+   && num_outputs == 1) {
memset(, 0, sizeof(mplsElem));
mplsElem.tag = SFLFLOW_EX_MPLS;
dpif_sflow_encode_mpls_stack(_stack,
diff --git a/ofproto/ofproto-dpif-sflow.h

[ovs-dev] [PATCH 1/3] ofproto-dpif-sflow: propagate actions within clone

2017-12-05 Thread Zoltan Balogh 
By avoiding Tx recirculation and embracing tnl_push action within clone,
the tunnel metadata is not propagated. Unless clone action is handled in
the dpif_sflow_read_actions() function as well. This commit resolves
this issue.
In addition, some sflow data needs to be stored and restored in
ofproto-dpif-xlate when native_tunnel_output() is invoked. Otherwise the
output action of underlay bridge is getting counted as well if sFlow is
set on the overlay bridge.

Signed-off-by: Zoltan Balogh 
CC: Sugesh Chandran 
---
 ofproto/ofproto-dpif-sflow.c  | 19 +--
 ofproto/ofproto-dpif-sflow.h  |  2 +-
 ofproto/ofproto-dpif-upcall.c |  2 +-
 ofproto/ofproto-dpif-xlate.c  |  7 +++
 tests/ofproto-dpif.at |  2 +-
 5 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index ccf89645b..0bdb3fdfa 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -961,7 +961,7 @@ sflow_read_set_action(const struct nlattr *attr,
 } else {
 dpif_sflow_read_actions(NULL,
 nl_attr_get(attr), nl_attr_get_size(attr),
-sflow_actions);
+sflow_actions, true);
 }
 break;
 case OVS_KEY_ATTR_PRIORITY:
@@ -1089,7 +1089,7 @@ dpif_sflow_capture_input_mpls(const struct flow *flow,
 void
 dpif_sflow_read_actions(const struct flow *flow,
const struct nlattr *actions, size_t actions_len,
-   struct dpif_sflow_actions *sflow_actions)
+   struct dpif_sflow_actions *sflow_actions, bool 
capture_mpls)
 {
 const struct nlattr *a;
 unsigned int left;
@@ -1099,7 +1099,7 @@ dpif_sflow_read_actions(const struct flow *flow,
return;
 }
 
-if (flow != NULL) {
+if (flow != NULL && capture_mpls == true) {
/* Make sure the MPLS output stack
 * is seeded with the input stack.
 */
@@ -1197,8 +1197,13 @@ dpif_sflow_read_actions(const struct flow *flow,
 * structure to report this.
 */
break;
+case OVS_ACTION_ATTR_CLONE:
+if (flow != NULL) {
+dpif_sflow_read_actions(flow, nl_attr_get(a), nl_attr_get_size(a),
+sflow_actions, false);
+}
+break;
case OVS_ACTION_ATTR_SAMPLE:
-   case OVS_ACTION_ATTR_CLONE:
 case OVS_ACTION_ATTR_ENCAP_NSH:
 case OVS_ACTION_ATTR_DECAP_NSH:
case OVS_ACTION_ATTR_UNSPEC:
@@ -1266,6 +1271,7 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
dp_packet *packet,
 struct dpif_sflow_port *in_dsp;
 struct dpif_sflow_port *out_dsp;
 ovs_be16 vlan_tci;
+uint32_t num_outputs;
 
 ovs_mutex_lock();
 sampler = ds->sflow_agent->samplers;
@@ -1333,11 +1339,12 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
dp_packet *packet,
}
 }
 
+num_outputs = dpif_sflow_cookie_num_outputs(cookie);
 /* Output tunnel. */
 if (sflow_actions
&& sflow_actions->encap_depth == 1
&& !sflow_actions->tunnel_err
-   && dpif_sflow_cookie_num_outputs(cookie) == 1) {
+   && (num_outputs == 1 || num_outputs == 2)) { /* push_tnl or clone + 
push_tnl */
tnlOutProto = sflow_actions->tunnel_ipproto;
if (tnlOutProto == 0) {
/* Try to infer the ip-protocol from the output port. */
@@ -1367,7 +1374,7 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct 
dp_packet *packet,
 if (sflow_actions
&& sflow_actions->mpls_stack_depth > 0
&& !sflow_actions->mpls_err
-   && dpif_sflow_cookie_num_outputs(cookie) == 1) {
+   && num_outputs == 1) {
memset(, 0, sizeof(mplsElem));
mplsElem.tag = SFLFLOW_EX_MPLS;
dpif_sflow_encode_mpls_stack(_stack,
diff --git a/ofproto/ofproto-dpif-sflow.h b/ofproto/ofproto-dpif-sflow.h
index 014e6cce3..01dd7d590 100644
--- a/ofproto/ofproto-dpif-sflow.h
+++ b/ofproto/ofproto-dpif-sflow.h
@@ -71,7 +71,7 @@ void dpif_sflow_wait(struct dpif_sflow *);
 
 void dpif_sflow_read_actions(const struct flow *,
 const struct nlattr *actions, size_t actions_len,
-struct dpif_sflow_actions *);
+struct dpif_sflow_actions *, bool capture_mpls);
 
 void dpif_sflow_received(struct dpif_sflow *, const struct dp_packet *,
  const struct flow *, odp_port_t odp_port,
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 02cf5415b..dc496344f 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1317,7 +1317,7 @@ dpif_read_actions(struct udpif *udpif, struct upcall 
*upcall,
 
 switch (type) {
 case SFLOW_UPCALL:
-dpif_sflow_read_actions(flow, actions, actions_len,