Hi Martin,

Somehow I didn’t receive this patch email via the ovs-dev mailing list, perhaps 
one of the many spam filters on the way interfered. Don't know if this response 
email will be recognized by ovs patchwork.

The nsh.at lines are broken wrongly in this email, but they look OK in 
patchworks.
Otherwise, LGTM.

Acked-by: Jan Scheurich <[email protected]>

/Jan

> -----Original Message-----
> From: Varghese, Martin (Nokia - IN/Bangalore)
> <[email protected]>
> Sent: Monday, 3 May, 2021 15:25
> To: Jan Scheurich <[email protected]>
> Subject: RE: [PATCH] Fix redundant datapath set ethernet action with NSH
> Decap
> 
> Hi Jan,
> 
> Could you please review this patch.
> 
> Regards,
> Martin
> 
> -----Original Message-----
> From: Martin Varghese <[email protected]>
> Sent: Tuesday, April 27, 2021 6:13 PM
> To: [email protected]; [email protected];
> [email protected]
> Cc: Varghese, Martin (Nokia - IN/Bangalore) <[email protected]>
> Subject: [PATCH] Fix redundant datapath set ethernet action with NSH Decap
> 
> From: Martin Varghese <[email protected]>
> 
> When a decap action is applied on NSH header encapsulatiing a ethernet
> packet a redundant set mac address action is programmed to the datapath.
> 
> Fixes: f839892a206a ("OF support and translation of generic encap and
> decap")
> Signed-off-by: Martin Varghese <[email protected]>
> ---
>  lib/odp-util.c               | 3 ++-
>  ofproto/ofproto-dpif-xlate.c | 2 ++
>  tests/nsh.at                 | 8 ++++----
>  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/odp-util.c b/lib/odp-util.c index e1199d1da..9d558082f 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -7830,7 +7830,8 @@ commit_set_ether_action(const struct flow *flow,
> struct flow *base_flow,
>      struct offsetof_sizeof ovs_key_ethernet_offsetof_sizeof_arr[] =
>          OVS_KEY_ETHERNET_OFFSETOF_SIZEOF_ARR;
> 
> -    if (flow->packet_type != htonl(PT_ETH)) {
> +    if ((flow->packet_type != htonl(PT_ETH)) ||
> +        (base_flow->packet_type != htonl(PT_ETH))) {
>          return;
>      }
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index
> 7108c8a30..a6f4ea334 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -6549,6 +6549,8 @@ xlate_generic_decap_action(struct xlate_ctx *ctx,
>                   * Delay generating pop_eth to the next commit. */
>                  flow->packet_type = htonl(PACKET_TYPE(OFPHTN_ETHERTYPE,
>                                                        ntohs(flow->dl_type)));
> +                flow->dl_src = eth_addr_zero;
> +                flow->dl_dst = eth_addr_zero;
>                  ctx->wc->masks.dl_type = OVS_BE16_MAX;
>              }
>              return false;
> diff --git a/tests/nsh.at b/tests/nsh.at index d5c772ff0..e84134e42 100644
> --- a/tests/nsh.at
> +++ b/tests/nsh.at
> @@ -105,7 +105,7 @@ bridge("br0")
> 
>  Final flow:
> in_port=1,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=11:22:33:44:55:6
> 6,dl_type=0x894f,nsh_flags=0,nsh_ttl=63,nsh_mdtype=1,nsh_np=3,nsh_spi=0
> x1234,nsh_si=255,nsh_c1=0x11223344,nsh_c2=0x0,nsh_c3=0x0,nsh_c4=0x0,
> nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
>  Megaflow:
> recirc_id=0,eth,ip,in_port=1,dl_dst=66:77:88:99:aa:bb,nw_frag=no
> -Datapath actions:
> push_nsh(flags=0,ttl=63,mdtype=1,np=3,spi=0x1234,si=255,c1=0x11223344,c
> 2=0x0,c3=0x0,c4=0x0),push_eth(src=00:00:00:00:00:00,dst=11:22:33:44:55:6
> 6),pop_eth,pop_nsh(),set(eth(dst=11:22:33:44:55:66)),recirc(0x1)
> +Datapath actions:
> +push_nsh(flags=0,ttl=63,mdtype=1,np=3,spi=0x1234,si=255,c1=0x11223344,
> c
> +2=0x0,c3=0x0,c4=0x0),push_eth(src=00:00:00:00:00:00,dst=11:22:33:44:55:
> +66),pop_eth,pop_nsh(),recirc(0x1)
>  ])
> 
>  AT_CHECK([
> @@ -139,7 +139,7 @@ ovs-appctl time/warp 1000  AT_CHECK([
>      ovs-appctl dpctl/dump-flows dummy@ovs-dummy | strip_used | grep -v
> ipv6 | sort  ], [0], [flow-dump from the main thread:
> -
> recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(dst=1e:2c:e9:2a:66:9e),eth
> _type(0x0800),ipv4(frag=no), packets:1, bytes:98, used:0.0s,
> actions:push_nsh(flags=0,ttl=63,mdtype=1,np=3,spi=0x1234,si=255,c1=0x112
> 23344,c2=0x0,c3=0x0,c4=0x0),push_eth(src=00:00:00:00:00:00,dst=11:22:33:
> 44:55:66),pop_eth,pop_nsh(),set(eth(dst=11:22:33:44:55:66)),recirc(0x3)
> +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(dst=1e:2c:e9:2a:66:9
> +e),eth_type(0x0800),ipv4(frag=no), packets:1, bytes:98, used:0.0s,
> +actions:push_nsh(flags=0,ttl=63,mdtype=1,np=3,spi=0x1234,si=255,c1=0x11
> +223344,c2=0x0,c3=0x0,c4=0x0),push_eth(src=00:00:00:00:00:00,dst=11:22:
> 3
> +3:44:55:66),pop_eth,pop_nsh(),recirc(0x3)
> 
> recirc_id(0x3),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=
> no), packets:1, bytes:98, used:0.0s, actions:2
>  ])
> 
> @@ -232,7 +232,7 @@ bridge("br0")
> 
>  Final flow:
> in_port=1,vlan_tci=0x0000,dl_src=00:00:00:00:00:00,dl_dst=11:22:33:44:55:6
> 6,dl_type=0x894f,nsh_flags=0,nsh_ttl=63,nsh_mdtype=2,nsh_np=3,nsh_spi=0
> x1234,nsh_si=255,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=0
>  Megaflow:
> recirc_id=0,eth,ip,in_port=1,dl_dst=66:77:88:99:aa:bb,nw_frag=no
> -Datapath actions:
> push_nsh(flags=0,ttl=63,mdtype=2,np=3,spi=0x1234,si=255,md2=0x10000a04
> 1234567820001408fedcba9876543210),push_eth(src=00:00:00:00:00:00,dst=
> 11:22:33:44:55:66),pop_eth,pop_nsh(),set(eth(dst=11:22:33:44:55:66)),recirc(
> 0x1)
> +Datapath actions:
> +push_nsh(flags=0,ttl=63,mdtype=2,np=3,spi=0x1234,si=255,md2=0x10000a0
> 41
> +234567820001408fedcba9876543210),push_eth(src=00:00:00:00:00:00,dst=
> 11:
> +22:33:44:55:66),pop_eth,pop_nsh(),recirc(0x1)
>  ])
> 
>  AT_CHECK([
> @@ -266,7 +266,7 @@ ovs-appctl time/warp 1000  AT_CHECK([
>      ovs-appctl dpctl/dump-flows dummy@ovs-dummy | strip_used | grep -v
> ipv6 | sort  ], [0], [flow-dump from the main thread:
> -
> recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(dst=1e:2c:e9:2a:66:9e),eth
> _type(0x0800),ipv4(frag=no), packets:1, bytes:98, used:0.0s,
> actions:push_nsh(flags=0,ttl=63,mdtype=2,np=3,spi=0x1234,si=255,md2=0x1
> 0000a041234567820001408fedcba9876543210),push_eth(src=00:00:00:00:0
> 0:00,dst=11:22:33:44:55:66),pop_eth,pop_nsh(),set(eth(dst=11:22:33:44:55:6
> 6)),recirc(0x3)
> +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(dst=1e:2c:e9:2a:66:9
> +e),eth_type(0x0800),ipv4(frag=no), packets:1, bytes:98, used:0.0s,
> +actions:push_nsh(flags=0,ttl=63,mdtype=2,np=3,spi=0x1234,si=255,md2=0x
> 1
> +0000a041234567820001408fedcba9876543210),push_eth(src=00:00:00:00:
> 00:00
> +,dst=11:22:33:44:55:66),pop_eth,pop_nsh(),recirc(0x3)
> 
> recirc_id(0x3),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=
> no), packets:1, bytes:98, used:0.0s, actions:2
>  ])
> 
> --
> 2.18.4

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to