[ovs-dev] [PATCH ovn v2 2/2] Add sctp_abort logical flow action.

Numan Siddique numans at ovn.org
Tue Jan 12 09:53:27 UTC 2021


On Tue, Jan 12, 2021 at 12:15 AM Mark Michelson <mmichels at redhat.com> wrote:

> This is used in similar situations as tcp_reset, only for SCTP flows.
>
> The "router port unreachable" test has been expanded to test SCTP. In
> doing so, two test cases were removed because they specified SCTP as the
> protocol and expected ICMP replies.
>
> Signed-off-by: Mark Michelson <mmichels at redhat.com>
>

Hi Mark,

The patch LGTM. But it has missed out on documenting the logical flows in
ovn-northd.8.xml.

Thanks
Numan


> ---
>  controller/pinctrl.c  |  5 +++
>  include/ovn/actions.h |  7 ++++
>  lib/actions.c         | 22 ++++++++++++
>  northd/ovn-northd.c   | 28 +++++++++++++--
>  tests/ovn.at          | 84 +++++++++++++++++++++++++++++++++++++++++--
>  utilities/ovn-trace.c |  5 +++
>  6 files changed, 147 insertions(+), 4 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index ce11aa365..b9880e257 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -3057,6 +3057,11 @@ process_packet_in(struct rconn *swconn, const
> struct ofp_header *msg)
>                                   &userdata, false);
>          break;
>
> +    case ACTION_OPCODE_SCTP_ABORT:
> +        pinctrl_handle_sctp_abort(swconn, &headers, &packet,
> +                                  &pin.flow_metadata, &userdata, false);
> +        break;
> +
>      case ACTION_OPCODE_REJECT:
>          pinctrl_handle_reject(swconn, &headers, &packet,
> &pin.flow_metadata,
>                                &userdata);
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index d104d4d64..be87e6135 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -106,6 +106,7 @@ struct ovn_extend_table;
>      OVNACT(CHK_LB_HAIRPIN_REPLY, ovnact_result)       \
>      OVNACT(CT_SNAT_TO_VIP,    ovnact_null)            \
>      OVNACT(BFD_MSG,           ovnact_null)            \
> +    OVNACT(SCTP_ABORT,        ovnact_nest)            \
>
>  /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */
>  enum OVS_PACKED_ENUM ovnact_type {
> @@ -634,6 +635,12 @@ enum action_opcode {
>       *  The actions, in OpenFlow 1.3 format, follow the action_header.
>       */
>      ACTION_OPCODE_BFD_MSG,
> +
> +    /* "sctp_abort { ...actions... }".
> +     *
> +     * The actions, in OpenFlow 1.3 format, follow the action_header.
> +     */
> +    ACTION_OPCODE_SCTP_ABORT,
>  };
>
>  /* Header. */
> diff --git a/lib/actions.c b/lib/actions.c
> index 86be97f44..56b8fab62 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1490,6 +1490,12 @@ parse_TCP_RESET(struct action_context *ctx)
>      parse_nested_action(ctx, OVNACT_TCP_RESET, "tcp", ctx->scope);
>  }
>
> +static void
> +parse_SCTP_ABORT(struct action_context *ctx)
> +{
> +    parse_nested_action(ctx, OVNACT_SCTP_ABORT, "sctp", ctx->scope);
> +}
> +
>  static void
>  parse_ND_NA(struct action_context *ctx)
>  {
> @@ -1571,6 +1577,12 @@ format_TCP_RESET(const struct ovnact_nest *nest,
> struct ds *s)
>      format_nested_action(nest, "tcp_reset", s);
>  }
>
> +static void
> +format_SCTP_ABORT(const struct ovnact_nest *nest, struct ds *s)
> +{
> +    format_nested_action(nest, "sctp_abort", s);
> +}
> +
>  static void
>  format_ND_NA(const struct ovnact_nest *nest, struct ds *s)
>  {
> @@ -1700,6 +1712,14 @@ encode_TCP_RESET(const struct ovnact_nest *on,
>      encode_nested_actions(on, ep, ACTION_OPCODE_TCP_RESET, ofpacts);
>  }
>
> +static void
> +encode_SCTP_ABORT(const struct ovnact_nest *on,
> +                  const struct ovnact_encode_params *ep,
> +                  struct ofpbuf *ofpacts)
> +{
> +    encode_nested_actions(on, ep, ACTION_OPCODE_SCTP_ABORT, ofpacts);
> +}
> +
>  static void
>  encode_REJECT(const struct ovnact_nest *on,
>                const struct ovnact_encode_params *ep,
> @@ -3837,6 +3857,8 @@ parse_action(struct action_context *ctx)
>          ovnact_put_IGMP(ctx->ovnacts);
>      } else if (lexer_match_id(ctx->lexer, "tcp_reset")) {
>          parse_TCP_RESET(ctx);
> +    } else if (lexer_match_id(ctx->lexer, "sctp_abort")) {
> +        parse_SCTP_ABORT(ctx);
>      } else if (lexer_match_id(ctx->lexer, "nd_na")) {
>          parse_ND_NA(ctx);
>      } else if (lexer_match_id(ctx->lexer, "nd_na_router")) {
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 50507685b..332dbc112 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -10529,7 +10529,7 @@ build_ipv6_input_flows_for_lrouter_port(
>                                    &op->nbrp->header_, lflows);
>          }
>
> -        /* UDP/TCP port unreachable */
> +        /* UDP/TCP/SCTP port unreachable */
>          if (!smap_get(&op->od->nbr->options, "chassis")
>              && !op->od->l3dgw_port) {
>              for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
> @@ -10545,6 +10545,18 @@ build_ipv6_input_flows_for_lrouter_port(
>                                          80, ds_cstr(match), action,
>                                          &op->nbrp->header_);
>
> +                ds_clear(match);
> +                ds_put_format(match,
> +                              "ip6 && ip6.dst == %s && !ip.later_frag &&
> sctp",
> +                              op->lrp_networks.ipv6_addrs[i].addr_s);
> +                action = "sctp_abort {"
> +                         "eth.dst <-> eth.src; "
> +                         "ip6.dst <-> ip6.src; "
> +                         "next; };";
> +                ovn_lflow_add_with_hint(lflows, op->od,
> S_ROUTER_IN_IP_INPUT,
> +                                        80, ds_cstr(match), action,
> +                                        &op->nbrp->header_);
> +
>                  ds_clear(match);
>                  ds_put_format(match,
>                                "ip6 && ip6.dst == %s && !ip.later_frag &&
> udp",
> @@ -10806,7 +10818,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
>
>          if (!smap_get(&op->od->nbr->options, "chassis")
>              && !op->od->l3dgw_port) {
> -            /* UDP/TCP port unreachable. */
> +            /* UDP/TCP/SCTP port unreachable. */
>              for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
>                  ds_clear(match);
>                  ds_put_format(match,
> @@ -10835,6 +10847,18 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
>                                          80, ds_cstr(match), action,
>                                          &op->nbrp->header_);
>
> +                ds_clear(match);
> +                ds_put_format(match,
> +                              "ip4 && ip4.dst == %s && !ip.later_frag &&
> sctp",
> +                              op->lrp_networks.ipv4_addrs[i].addr_s);
> +                action = "sctp_abort {"
> +                         "eth.dst <-> eth.src; "
> +                         "ip4.dst <-> ip4.src; "
> +                         "next; };";
> +                ovn_lflow_add_with_hint(lflows, op->od,
> S_ROUTER_IN_IP_INPUT,
> +                                        80, ds_cstr(match), action,
> +                                        &op->nbrp->header_);
> +
>                  ds_clear(match);
>                  ds_put_format(match,
>                                "ip4 && ip4.dst == %s && !ip.later_frag",
> diff --git a/tests/ovn.at b/tests/ovn.at
> index c71e81822..f575eecb5 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1637,6 +1637,17 @@ tcp_reset { };
>      encodes as controller(userdata=00.00.00.0b.00.00.00.00)
>      has prereqs tcp
>
> +# sctp_abort
> +sctp_abort {eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
> +    formats as sctp_abort { eth.dst = ff:ff:ff:ff:ff:ff; output; };
> output;
> +    encodes as
> controller(userdata=00.00.00.18.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64)
> +    has prereqs sctp
> +
> +sctp_abort { };
> +    formats as sctp_abort { drop; };
> +    encodes as controller(userdata=00.00.00.18.00.00.00.00)
> +    has prereqs sctp
> +
>  # reject
>  reject { eth.dst = ff:ff:ff:ff:ff:ff; output; }; output;
>      encodes as
> controller(userdata=00.00.00.16.00.00.00.00.00.19.00.10.80.00.06.06.ff.ff.ff.ff.ff.ff.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00),resubmit(,64)
> @@ -14354,6 +14365,45 @@ test_tcp_syn_packet() {
>      as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
>  }
>
> +# test_sctp_init_packet INPORT HV ETH_SRC ETH_DST IPV4_SRC IPV4_DST
> IP_CHKSUM SCTP_SPORT SCTP_DPORT SCTP_INIT_TAG SCTP_CHKSUM EXP_IP_CHKSUM
> EXP_SCTP_ABORT_CHKSUM
> +#
> +# Causes a packet to be received on INPORT of the hypervisor HV. The
> packet is an SCTP INIT chunk with
> +# ETH_SRC, ETH_DST, IPV4_SRC, IPV4_DST, IP_CHKSUM, SCTP_SPORT,
> SCTP_DPORT, and SCTP_CHKSUM as specified.
> +# The INIT "initiate_tag" will be set to SCTP_INIT_TAG.
> +# EXP_IP_CHKSUM and EXP_SCTP_CHKSUM are the ip and sctp checksums of the
> SCTP ABORT chunk generated by OVN logical router
> +#
> +# INPORT is an lport number, e.g. 1 for vif1.
> +# HV is a hypervisor number.
> +# ETH_SRC and ETH_DST are each 12 hex digits.
> +# IPV4_SRC and IPV4_DST are each 8 hex digits.
> +# SCTP_SPORT and SCTP_DPORT are 4 hex digits.
> +# IP_CHKSUM and EXP_IP_CHKSUM are 4 hex digits.
> +# SCTP_CHKSUM and EXP_SCTP_CHKSUM are 8 hex digits.
> +test_sctp_init_packet() {
> +    local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 ipv4_dst=$6
> ip_chksum=$7
> +    local sctp_sport=$8 sctp_dport=$9 sctp_init_tag=${10}
> sctp_chksum=${11}
> +    local exp_ip_chksum=${12} exp_sctp_abort_chksum=${13}
> +
> +    local ip_ttl=ff
> +    local eth_hdr=${eth_dst}${eth_src}0800
> +    local
> ip_hdr=4500002500004000${ip_ttl}84${ip_chksum}${ipv4_src}${ipv4_dst}
> +    local sctp_hdr=${sctp_sport}${sctp_dport}00000000${sctp_chksum}
> +    local
> sctp_init=01000014${sctp_init_tag}0000000000010001${sctp_init_tag}
> +
> +    local packet=${eth_hdr}${ip_hdr}${sctp_hdr}${sctp_init}
> +
> +    local sctp_abort_ttl=3e
> +    local reply_eth_hdr=${eth_src}${eth_dst}0800
> +    local
> reply_ip_hdr=4500002400004000${sctp_abort_ttl}84${exp_ip_chksum}${ipv4_dst}${ipv4_src}
> +    local
> reply_sctp_hdr=${sctp_dport}${sctp_sport}${sctp_init_tag}${exp_sctp_abort_chksum}
> +    local reply_sctp_abort=06000004
> +
> +    local
> reply=${reply_eth_hdr}${reply_ip_hdr}${reply_sctp_hdr}${reply_sctp_abort}
> +    echo $reply >> vif$inport.expected
> +
> +    check as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
> +}
> +
>  # test_tcp6_packet INPORT HV ETH_SRC ETH_DST IPV6_SRC IPV6_ROUTER
> TCP_SPORT TCP_DPORT TCP_CHKSUM EXP_TCP_RST_CHKSUM
>  #
>  # Causes a packet to be received on INPORT of the hypervisor HV. The
> packet is a TCP syn segment with
> @@ -14374,6 +14424,36 @@ test_tcp6_packet() {
>      as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
>  }
>
> +# test_tcp6_packet INPORT HV ETH_SRC ETH_DST IPV6_SRC IPV6_ROUTER
> SCTP_SPORT SCTP_DPORT SCTP_INIT_TAG SCTP_CHKSUM EXP_SCTP_ABORT_CHKSUM
> +#
> +# Causes a packet to be received on INPORT of the hypervisor HV. The
> packet is an SCTP INIT chunk with
> +# ETH_SRC, ETH_DST, IPV6_SRC, IPV6_ROUTER, SCTP_SPORT, SCTP_DPORT and
> SCTP_CHKSUM as specified.
> +# The INIT "initiate_tag" will be set to SCTP_INIT_TAG.
> +# EXP_SCTP_CHKSUM is the sctp checksum of the SCTP ABORT chunk generated
> by OVN logical router
> +test_sctp6_packet() {
> +    local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv6_src=$5 ipv6_router=$6
> +    local sctp_sport=$7 sctp_dport=$8 sctp_init_tag=$9 sctp_chksum=${10}
> +    local exp_sctp_abort_chksum=${11}
> +    shift 11
> +
> +    local eth_hdr=${eth_dst}${eth_src}86dd
> +    local ip_hdr=60000000002084ff${ipv6_src}${ipv6_router}
> +    local sctp_hdr=${sctp_sport}${sctp_dport}00000000${sctp_chksum}
> +    local
> sctp_init=01000014${sctp_init_tag}0000000000010001${sctp_init_tag}
> +
> +    local packet=${eth_hdr}${ip_hdr}${sctp_hdr}${sctp_init}
> +
> +    local reply_eth_hdr=${eth_src}${eth_dst}86dd
> +    local reply_ip_hdr=600000000010843e${ipv6_router}${ipv6_src}
> +    local
> reply_sctp_hdr=${sctp_dport}${sctp_sport}${sctp_init_tag}${exp_sctp_abort_chksum}
> +    local reply_sctp_abort=06000004
> +
> +    local
> reply=${reply_eth_hdr}${reply_ip_hdr}${reply_sctp_hdr}${reply_sctp_abort}
> +    echo $reply >> vif$inport.expected
> +
> +    check as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
> +}
> +
>  # test_ip6_packet INPORT HV ETH_SRC ETH_DST IPV6_SRC IPV6_DST IPV6_PROTO
> IPV6_LEN DATA EXP_ICMP_CODE EXP_ICMP_CHKSUM
>  #
>  # Causes a packet to be received on INPORT of the hypervisor HV. The
> packet is an IPv6
> @@ -14428,13 +14508,13 @@ OVN_POPULATE_ARP
>  ovn-nbctl --wait=hv sync
>
>  test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1)
> $(ip_to_hex 192 168 1 254) 11 0000 f87c f485 0303
> -test_ip_packet 1 1 000000000001 00000000ff01 $(ip_to_hex 192 168 1 1)
> $(ip_to_hex 192 168 1 254) 84 0000 f87c f413 0302
>  test_ip6_packet 1 1 000000000001 00000000ff01
> 20010db8000100000000000000000011 20010db8000100000000000000000001 11 0015
> dbb8303900155bac6b646f65206676676e6d66720a 0104 1d31
>  OVN_CHECK_PACKETS([hv1/vif1-tx.pcap], [vif1.expected])
>
>  test_tcp_syn_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2
> 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 0000 b680 6e05
> -test_ip6_packet 2 2 000000000002 00000000ff02
> 20010db8000200000000000000000011 20010db8000200000000000000000001 84 0004
> 01020304 0103 5e74
> +test_sctp_init_packet 2 2 000000000002 00000000ff02 $(ip_to_hex 192 168 2
> 1) $(ip_to_hex 192 168 2 254) 0000 8b40 3039 00000001 82112601 b606 10fe95b6
>  test_tcp6_packet 2 2 000000000002 00000000ff02
> 20010db8000200000000000000000011 20010db8000200000000000000000001 8b40 3039
> 0000 98cd
> +test_sctp6_packet 2 2 000000000002 00000000ff02
> 20010db8000200000000000000000011 20010db8000200000000000000000001 8b40 3039
> 00000002 C0379D5A 39f23aaf
>  OVN_CHECK_PACKETS([hv2/vif2-tx.pcap], [vif2.expected])
>
>  OVN_CLEANUP([hv1], [hv2])
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index d54ffbcd9..c0a0edc0a 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -2594,6 +2594,11 @@ trace_actions(const struct ovnact *ovnacts, size_t
> ovnacts_len,
>                                false, pipeline, super);
>              break;
>
> +        case OVNACT_SCTP_ABORT:
> +            execute_sctp_abort(ovnact_get_SCTP_ABORT(a), dp, uflow,
> table_id,
> +                               false, pipeline, super);
> +            break;
> +
>          case OVNACT_OVNFIELD_LOAD:
>              execute_ovnfield_load(ovnact_get_OVNFIELD_LOAD(a), super);
>              break;
> --
> 2.29.2
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list