| Message ID | 1512470596-18091-2-git-send-email-zoltan.balogh@ericsson.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix tunnel neighbor cache population | expand |
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 <zoltan.balogh@ericsson.com> > CC: Sugesh Chandran <sugesh.chandran@intel.com> > --- > 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(&mutex); > 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(&mplsElem, 0, sizeof(mplsElem)); > mplsElem.tag = SFLFLOW_EX_MPLS; > dpif_sflow_encode_mpls_stack(&mplsElem.flowType.mpls.out_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, upcall_data); > + dpif_sflow_read_actions(flow, actions, actions_len, upcall_data, true); > break; > case FLOW_SAMPLE_UPCALL: > case IPFIX_UPCALL: > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index fcced344e..19343aca3 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -3259,6 +3259,9 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport, > char buf_sip6[INET6_ADDRSTRLEN]; > char buf_dip6[INET6_ADDRSTRLEN]; > > + /* Store sFlow data. */ > + uint32_t sflow_n_outputs = ctx->sflow_n_outputs; > + > /* Structures to backup Ethernet and IP of base_flow. */ > struct flow old_base_flow; > struct flow old_flow; > @@ -3410,6 +3413,10 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport, > /* Restore the flows after the translation. */ > memcpy(&ctx->xin->flow, &old_flow, sizeof ctx->xin->flow); > memcpy(&ctx->base_flow, &old_base_flow, sizeof ctx->base_flow); > + > + /* Restore sFlow data. */ > + ctx->sflow_n_outputs = sflow_n_outputs; > + > return 0; > } > > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at > index 9a51a1364..cdf48f4de 100644 > --- a/tests/ofproto-dpif.at > +++ b/tests/ofproto-dpif.at > @@ -6286,7 +6286,7 @@ AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK > AT_CHECK([ovs-ofctl add-flow br0 action=normal]) > > dnl Prime ARP Cache for 1.1.2.92 > -AT_CHECK([ovs-appctl netdev-dummy/receive br0 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)']) > +AT_CHECK([ovs-appctl netdev-dummy/receive p0 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)']) > > dnl configure sflow on int-br only > ovs-vsctl \
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(&mutex); 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(&mplsElem, 0, sizeof(mplsElem)); mplsElem.tag = SFLFLOW_EX_MPLS; dpif_sflow_encode_mpls_stack(&mplsElem.flowType.mpls.out_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, upcall_data); + dpif_sflow_read_actions(flow, actions, actions_len, upcall_data, true); break; case FLOW_SAMPLE_UPCALL: case IPFIX_UPCALL: diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index fcced344e..19343aca3 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3259,6 +3259,9 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport, char buf_sip6[INET6_ADDRSTRLEN]; char buf_dip6[INET6_ADDRSTRLEN]; + /* Store sFlow data. */ + uint32_t sflow_n_outputs = ctx->sflow_n_outputs; + /* Structures to backup Ethernet and IP of base_flow. */ struct flow old_base_flow; struct flow old_flow; @@ -3410,6 +3413,10 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct xport *xport, /* Restore the flows after the translation. */ memcpy(&ctx->xin->flow, &old_flow, sizeof ctx->xin->flow); memcpy(&ctx->base_flow, &old_base_flow, sizeof ctx->base_flow); + + /* Restore sFlow data. */ + ctx->sflow_n_outputs = sflow_n_outputs; + return 0; } diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 9a51a1364..cdf48f4de 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -6286,7 +6286,7 @@ AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK AT_CHECK([ovs-ofctl add-flow br0 action=normal]) dnl Prime ARP Cache for 1.1.2.92 -AT_CHECK([ovs-appctl netdev-dummy/receive br0 'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)']) +AT_CHECK([ovs-appctl netdev-dummy/receive p0 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)']) dnl configure sflow on int-br only ovs-vsctl \
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 <zoltan.balogh@ericsson.com> CC: Sugesh Chandran <sugesh.chandran@intel.com> --- 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(-)