[ovs-dev] [PATCH ovn v2 1/2] Implement SCTP-specific reject() action.

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


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

> Currently in OVN, if an SCTP packet hits a reject() action, OVN responds
> with an ICMP packet. Instead, we should send an SCTP packet with an
> ABORT chunk. This will either end the current association or will
> prevent an association from being created, depending on which stage of
> the SCTP state machine we currently are in.
>
> This patch adds the desired behavior for SCTP. The reject() action will
> now send an SCTP ABORT if the incoming packet is SCTP.
>
> Signed-off-by: Mark Michelson <mmichels at redhat.com>
>

Acked-by: Numan Siddique <numans at ovn.org>

Please see one small nit below. I don't mind if you want to ignore it.

Thanks
Numan


> ---
>  controller/pinctrl.c  | 113 ++++++++++++++++++++++++++++++++++++++++++
>  lib/ovn-util.h        |  33 ++++++++++++
>  tests/ovn.at          |  43 ++++++++++++++++
>  utilities/ovn-trace.c |  87 ++++++++++++++++++++++++++++++++
>  4 files changed, 276 insertions(+)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 62be94018..ce11aa365 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -38,6 +38,7 @@
>  #include "openvswitch/ofp-util.h"
>  #include "openvswitch/vlog.h"
>  #include "lib/random.h"
> +#include "lib/crc32c.h"
>
>  #include "lib/dhcp.h"
>  #include "ovn-controller.h"
> @@ -1781,6 +1782,116 @@ pinctrl_handle_tcp_reset(struct rconn *swconn,
> const struct flow *ip_flow,
>      dp_packet_uninit(&packet);
>  }
>
> +static void dp_packet_put_sctp_abort(struct dp_packet *packet,
> +                                     bool reflect_tag)
> +{
> +    struct sctp_chunk_header abort = {
> +        .sctp_chunk_type = SCTP_CHUNK_TYPE_ABORT,
> +        .sctp_chunk_flags = reflect_tag ? SCTP_ABORT_CHUNK_FLAG_T : 0,
> +        .sctp_chunk_len = htons(SCTP_CHUNK_HEADER_LEN),
> +    };
> +
> +    dp_packet_put(packet, &abort, sizeof abort);
> +}
> +
> +static void
> +pinctrl_handle_sctp_abort(struct rconn *swconn, const struct flow
> *ip_flow,
> +                         struct dp_packet *pkt_in,
> +                         const struct match *md, struct ofpbuf *userdata,
> +                         bool loopback)
> +{
> +    if (ip_flow->nw_proto != IPPROTO_SCTP) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +        VLOG_WARN_RL(&rl, "SCTP_ABORT action on non-SCTP packet");
> +        return;
> +    }
> +
> +    struct sctp_header *sh_in = dp_packet_l4(pkt_in);
> +    if (!sh_in) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +        VLOG_WARN_RL(&rl, "SCTP_ABORT action on malformed SCTP packet");
> +        return;
> +    }
> +
> +    const struct sctp_chunk_header *sh_in_chunk =
> +        dp_packet_get_sctp_payload(pkt_in);
> +    if (!sh_in_chunk) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +        VLOG_WARN_RL(&rl, "SCTP_ABORT action on SCTP packet with no
> chunks");
> +        return;
> +    }
> +
> +    if (sh_in_chunk->sctp_chunk_type == SCTP_CHUNK_TYPE_ABORT) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +        VLOG_WARN_RL(&rl, "sctp_abort action on incoming SCTP ABORT.");
> +        return;
> +    }
> +
> +    const struct sctp_init_chunk *sh_in_init = NULL;
>

You can declare the above variable 'sh_in_init' inside the if since it is
not used
outside of this 'if'.



> +    if (sh_in_chunk->sctp_chunk_type == SCTP_CHUNK_TYPE_INIT) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +        sh_in_init = dp_packet_at(pkt_in, pkt_in->l4_ofs +
> +                                          SCTP_HEADER_LEN +
> +                                          SCTP_CHUNK_HEADER_LEN,
> +                                  SCTP_INIT_CHUNK_LEN);
> +        if (!sh_in_init) {
> +            VLOG_WARN_RL(&rl, "Incomplete SCTP INIT chunk. Ignoring
> packet.");
> +            return;
> +        }
> +    }
> +
> +    uint64_t packet_stub[128 / 8];
> +    struct dp_packet packet;
> +
> +    dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
> +
> +    struct eth_addr eth_src = loopback ? ip_flow->dl_dst :
> ip_flow->dl_src;
> +    struct eth_addr eth_dst = loopback ? ip_flow->dl_src :
> ip_flow->dl_dst;
> +
> +    if (get_dl_type(ip_flow) == htons(ETH_TYPE_IPV6)) {
> +        const struct in6_addr *ip6_src =
> +            loopback ? &ip_flow->ipv6_dst : &ip_flow->ipv6_src;
> +        const struct in6_addr *ip6_dst =
> +            loopback ? &ip_flow->ipv6_src : &ip_flow->ipv6_dst;
> +        pinctrl_compose_ipv6(&packet, eth_src, eth_dst,
> +                             (struct in6_addr *) ip6_src,
> +                             (struct in6_addr *) ip6_dst,
> +                             IPPROTO_SCTP, 63, SCTP_HEADER_LEN +
> +                                               SCTP_CHUNK_HEADER_LEN);
> +    } else {
> +        ovs_be32 nw_src = loopback ? ip_flow->nw_dst : ip_flow->nw_src;
> +        ovs_be32 nw_dst = loopback ? ip_flow->nw_src : ip_flow->nw_dst;
> +        pinctrl_compose_ipv4(&packet, eth_src, eth_dst, nw_src, nw_dst,
> +                             IPPROTO_SCTP, 63, SCTP_HEADER_LEN +
> +                                               SCTP_CHUNK_HEADER_LEN);
> +    }
> +
> +    struct sctp_header *sh = dp_packet_put_zeros(&packet, sizeof *sh);
> +    dp_packet_set_l4(&packet, sh);
> +    sh->sctp_dst = ip_flow->tp_src;
> +    sh->sctp_src = ip_flow->tp_dst;
> +    put_16aligned_be32(&sh->sctp_csum, 0);
> +
> +    bool tag_reflected;
> +    if (get_16aligned_be32(&sh_in->sctp_vtag) == 0 && sh_in_init) {
> +        /* See RFC 4960 Section 8.4, item 3. */
> +        put_16aligned_be32(&sh->sctp_vtag, sh_in_init->initiate_tag);
> +        tag_reflected = false;
> +    } else {
> +        /* See RFC 4960 Section 8.4, item 8. */
> +        sh->sctp_vtag = sh_in->sctp_vtag;
> +        tag_reflected = true;
> +    }
> +
> +    dp_packet_put_sctp_abort(&packet, tag_reflected);
> +
> +    put_16aligned_be32(&sh->sctp_csum, crc32c((void *) sh,
> +
> dp_packet_l4_size(&packet)));
> +
> +    set_actions_and_enqueue_msg(swconn, &packet, md, userdata);
> +    dp_packet_uninit(&packet);
> +}
> +
>  static void
>  pinctrl_handle_reject(struct rconn *swconn, const struct flow *ip_flow,
>                        struct dp_packet *pkt_in,
> @@ -1788,6 +1899,8 @@ pinctrl_handle_reject(struct rconn *swconn, const
> struct flow *ip_flow,
>  {
>      if (ip_flow->nw_proto == IPPROTO_TCP) {
>          pinctrl_handle_tcp_reset(swconn, ip_flow, pkt_in, md, userdata,
> true);
> +    } else if (ip_flow->nw_proto == IPPROTO_SCTP) {
> +        pinctrl_handle_sctp_abort(swconn, ip_flow, pkt_in, md, userdata,
> true);
>      } else {
>          pinctrl_handle_icmp(swconn, ip_flow, pkt_in, md, userdata, true,
> true);
>      }
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index 679f47a97..b604b073e 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -227,4 +227,37 @@ bool ip_address_and_port_from_lb_key(const char *key,
> char **ip_address,
>   * value. */
>  char *ovn_get_internal_version(void);
>
> +
> +/* OVN Packet definitions. These may eventually find a home in OVS's
> + * packets.h file. For the time being, they live here because OVN uses
> them
> + * and OVS does not.
> + */
> +#define SCTP_CHUNK_HEADER_LEN 4
> +struct sctp_chunk_header {
> +    uint8_t sctp_chunk_type;
> +    uint8_t sctp_chunk_flags;
> +    ovs_be16 sctp_chunk_len;
> +};
> +BUILD_ASSERT_DECL(SCTP_CHUNK_HEADER_LEN == sizeof(struct
> sctp_chunk_header));
> +
> +#define SCTP_INIT_CHUNK_LEN 16
> +struct sctp_init_chunk {
> +    ovs_be32 initiate_tag;
> +    ovs_be32 a_rwnd;
> +    ovs_be16 num_outbound_streams;
> +    ovs_be16 num_inbound_streams;
> +    ovs_be32 initial_tsn;
> +};
> +BUILD_ASSERT_DECL(SCTP_INIT_CHUNK_LEN == sizeof(struct sctp_init_chunk));
> +
> +/* These are the only SCTP chunk types that OVN cares about.
> + * There is no need to define the other chunk types until they are
> + * needed.
> + */
> +#define SCTP_CHUNK_TYPE_INIT  1
> +#define SCTP_CHUNK_TYPE_ABORT 6
> +
> +/* See RFC 4960 Sections 3.3.7 and 8.5.1 for information on this flag. */
> +#define SCTP_ABORT_CHUNK_FLAG_T (1 << 0)
> +
>  #endif
> diff --git a/tests/ovn.at b/tests/ovn.at
> index d50bdf9f7..c71e81822 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -12895,6 +12895,45 @@ test_tcp_syn_packet() {
>      check 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 from the ACL rule hit
> +#
> +# INPORT is an lport number, e.g. 11 for vif11.
> +# 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=3f
> +    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
> +}
> +
>  # Create hypervisors hv[123].
>  # Add vif1[123] to hv1, vif2[123] to hv2, vif3[123] to hv3.
>  # Add all of the vifs to a single logical switch sw0.
> @@ -12948,6 +12987,10 @@ test_tcp_syn_packet 11 1 000000000011
> 000000000021 $(ip_to_hex 192 168 1 11) $(i
>  test_tcp_syn_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1
> 21) $(ip_to_hex 192 168 1 11) 0000 8b40 3039 0000 b85f 70e4
>  test_tcp_syn_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1
> 31) $(ip_to_hex 192 168 1 12) 0000 8b40 3039 0000 b854 70d9
>
> +test_sctp_init_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168
> 1 11) $(ip_to_hex 192 168 1 21) 0000 8b40 3039 00000001 82112601 b7e5
> 10fe95b6
> +test_sctp_init_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168
> 1 21) $(ip_to_hex 192 168 1 11) 0000 8b40 3039 00000002 C0379D5A b7e5
> 39f23aaf
> +test_sctp_init_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168
> 1 31) $(ip_to_hex 192 168 1 12) 0000 8b40 3039 00000003 028E263C b7da
> 7124045b
> +
>  for i in 1 2 3; do
>      OVN_CHECK_PACKETS([hv$i/vif${i}1-tx.pcap], [vif${i}1.expected])
>  done
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index a81a5e936..d54ffbcd9 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -1806,6 +1806,91 @@ execute_tcp_reset(const struct ovnact_nest *on,
>          execute_tcp6_reset(on, dp, uflow, table_id, loopback, pipeline,
> super);
>      }
>  }
> +
> +static void
> +execute_sctp4_abort(const struct ovnact_nest *on,
> +                    const struct ovntrace_datapath *dp,
> +                    const struct flow *uflow, uint8_t table_id,
> +                    bool loopback, enum ovnact_pipeline pipeline,
> +                    struct ovs_list *super)
> +{
> +    struct flow sctp_flow = *uflow;
> +
> +    /* Update fields for TCP SCTP. */
> +    if (loopback) {
> +        sctp_flow.dl_dst = uflow->dl_src;
> +        sctp_flow.dl_src = uflow->dl_dst;
> +        sctp_flow.nw_dst = uflow->nw_src;
> +        sctp_flow.nw_src = uflow->nw_dst;
> +    } else {
> +        sctp_flow.dl_dst = uflow->dl_dst;
> +        sctp_flow.dl_src = uflow->dl_src;
> +        sctp_flow.nw_dst = uflow->nw_dst;
> +        sctp_flow.nw_src = uflow->nw_src;
> +    }
> +    sctp_flow.nw_proto = IPPROTO_SCTP;
> +    sctp_flow.nw_ttl = 255;
> +    sctp_flow.tp_src = uflow->tp_src;
> +    sctp_flow.tp_dst = uflow->tp_dst;
> +
> +    struct ovntrace_node *node = ovntrace_node_append(
> +        super, OVNTRACE_NODE_TRANSFORMATION, "sctp_abort");
> +
> +    trace_actions(on->nested, on->nested_len, dp, &sctp_flow,
> +                  table_id, pipeline, &node->subs);
> +}
> +
> +static void
> +execute_sctp6_abort(const struct ovnact_nest *on,
> +                    const struct ovntrace_datapath *dp,
> +                    const struct flow *uflow, uint8_t table_id,
> +                    bool loopback, enum ovnact_pipeline pipeline,
> +                    struct ovs_list *super)
> +{
> +    struct flow sctp_flow = *uflow;
> +
> +    /* Update fields for SCTP. */
> +    if (loopback) {
> +        sctp_flow.dl_dst = uflow->dl_src;
> +        sctp_flow.dl_src = uflow->dl_dst;
> +        sctp_flow.ipv6_dst = uflow->ipv6_src;
> +        sctp_flow.ipv6_src = uflow->ipv6_dst;
> +    } else {
> +        sctp_flow.dl_dst = uflow->dl_dst;
> +        sctp_flow.dl_src = uflow->dl_src;
> +        sctp_flow.ipv6_dst = uflow->ipv6_dst;
> +        sctp_flow.ipv6_src = uflow->ipv6_src;
> +    }
> +    sctp_flow.nw_proto = IPPROTO_TCP;
> +    sctp_flow.nw_ttl = 255;
> +    sctp_flow.tp_src = uflow->tp_src;
> +    sctp_flow.tp_dst = uflow->tp_dst;
> +    sctp_flow.tcp_flags = htons(TCP_RST);
> +
> +    struct ovntrace_node *node = ovntrace_node_append(
> +        super, OVNTRACE_NODE_TRANSFORMATION, "sctp_abort");
> +
> +    trace_actions(on->nested, on->nested_len, dp, &sctp_flow,
> +                  table_id, pipeline, &node->subs);
> +}
> +
> +static void
> +execute_sctp_abort(const struct ovnact_nest *on,
> +                   const struct ovntrace_datapath *dp,
> +                   const struct flow *uflow, uint8_t table_id,
> +                   bool loopback, enum ovnact_pipeline pipeline,
> +                   struct ovs_list *super)
> +{
> +    if (get_dl_type(uflow) == htons(ETH_TYPE_IP)) {
> +        execute_sctp4_abort(on, dp, uflow, table_id, loopback,
> +                            pipeline, super);
> +    } else {
> +        execute_sctp6_abort(on, dp, uflow, table_id, loopback,
> +                            pipeline, super);
> +    }
> +}
> +
> +
>  static void
>  execute_reject(const struct ovnact_nest *on,
>                 const struct ovntrace_datapath *dp,
> @@ -1814,6 +1899,8 @@ execute_reject(const struct ovnact_nest *on,
>  {
>      if (uflow->nw_proto == IPPROTO_TCP) {
>          execute_tcp_reset(on, dp, uflow, table_id, true, pipeline, super);
> +    } else if (uflow->nw_proto == IPPROTO_SCTP) {
> +        execute_sctp_abort(on, dp, uflow, table_id, true, pipeline,
> super);
>      } else {
>          if (get_dl_type(uflow) == htons(ETH_TYPE_IP)) {
>              execute_icmp4(on, dp, uflow, table_id, true, pipeline, super);
> --
> 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