[ovs-dev] [PATCH ovn v2 1/3] actions: Add a new OVN action - reject {}.

Dumitru Ceara dceara at redhat.com
Wed Oct 14 09:45:17 UTC 2020


On 10/14/20 11:15 AM, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
> 
> This action is similar to tcp_reset and icmp4/icmp6 action. If the matching
> packet is an IPv4 of IPv6 TCP packet, it sends out TCP RST packet else it
> sends out ICMPv4 or ICMPv6 Destination unreachable packet.
> 
> A future patch will make use of this action for reject ACL.
> 
> Signed-off-by: Numan Siddique <numans at ovn.org>
> ---
>  controller/pinctrl.c  | 17 +++++++++++++++++
>  include/ovn/actions.h |  9 ++++++++-
>  lib/actions.c         | 22 ++++++++++++++++++++++
>  ovn-sb.xml            | 10 ++++++++++
>  tests/ovn.at          | 12 ++++++++++++
>  utilities/ovn-trace.c | 22 ++++++++++++++++++++++
>  6 files changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 0a7020533b..631d058458 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -1670,6 +1670,18 @@ pinctrl_handle_tcp_reset(struct rconn *swconn, const struct flow *ip_flow,
>      dp_packet_uninit(&packet);
>  }
>  
> +static void
> +pinctrl_handle_reject(struct rconn *swconn, const struct flow *ip_flow,
> +                      struct dp_packet *pkt_in,
> +                      const struct match *md, struct ofpbuf *userdata)
> +{
> +    if (ip_flow->nw_proto == IPPROTO_TCP) {
> +        pinctrl_handle_tcp_reset(swconn, ip_flow, pkt_in, md, userdata);
> +    } else {
> +        pinctrl_handle_icmp(swconn, ip_flow, pkt_in, md, userdata, true);
> +    }
> +}
> +
>  static bool
>  is_dhcp_flags_broadcast(ovs_be16 flags)
>  {
> @@ -2787,6 +2799,11 @@ process_packet_in(struct rconn *swconn, const struct ofp_header *msg)
>                                   &userdata);
>          break;
>  
> +    case ACTION_OPCODE_REJECT:
> +        pinctrl_handle_reject(swconn, &headers, &packet, &pin.flow_metadata,
> +                              &userdata);
> +        break;
> +
>      case ACTION_OPCODE_PUT_ICMP4_FRAG_MTU:
>      case ACTION_OPCODE_PUT_ICMP6_FRAG_MTU:
>          pinctrl_handle_put_icmp_frag_mtu(swconn, &headers, &packet, &pin,
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 636cb4bc1a..b4e5acabb9 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -95,7 +95,8 @@ struct ovn_extend_table;
>      OVNACT(HANDLE_SVC_CHECK,  ovnact_handle_svc_check) \
>      OVNACT(FWD_GROUP,         ovnact_fwd_group)       \
>      OVNACT(DHCP6_REPLY,       ovnact_null)            \
> -    OVNACT(ICMP6_ERROR,       ovnact_nest)
> +    OVNACT(ICMP6_ERROR,       ovnact_nest)            \
> +    OVNACT(REJECT,            ovnact_nest)            \
>  
>  /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */
>  enum OVS_PACKED_ENUM ovnact_type {
> @@ -605,6 +606,12 @@ enum action_opcode {
>      /* MTU value (to put in the icmp6 header field - frag_mtu) follow the
>       * action header. */
>      ACTION_OPCODE_PUT_ICMP6_FRAG_MTU,
> +
> +    /* "reject { ...actions... }".
> +     *
> +     * The actions, in OpenFlow 1.3 format, follow the action_header.
> +     */
> +    ACTION_OPCODE_REJECT,
>  };
>  
>  /* Header. */
> diff --git a/lib/actions.c b/lib/actions.c
> index 5fe0a3897e..1e1bdeff24 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1386,6 +1386,12 @@ parse_CLONE(struct action_context *ctx)
>      parse_nested_action(ctx, OVNACT_CLONE, NULL, WR_DEFAULT);
>  }
>  
> +static void
> +parse_REJECT(struct action_context *ctx)
> +{
> +    parse_nested_action(ctx, OVNACT_REJECT, NULL, ctx->scope);
> +}
> +
>  static void
>  format_nested_action(const struct ovnact_nest *on, const char *name,
>                       struct ds *s)
> @@ -1479,6 +1485,12 @@ format_TRIGGER_EVENT(const struct ovnact_controller_event *event,
>      ds_put_cstr(s, ");");
>  }
>  
> +static void
> +format_REJECT(const struct ovnact_nest *nest, struct ds *s)
> +{
> +    format_nested_action(nest, "reject", s);
> +}
> +
>  static void
>  encode_nested_actions(const struct ovnact_nest *on,
>                        const struct ovnact_encode_params *ep,
> @@ -1560,6 +1572,14 @@ encode_TCP_RESET(const struct ovnact_nest *on,
>      encode_nested_actions(on, ep, ACTION_OPCODE_TCP_RESET, ofpacts);
>  }
>  
> +static void
> +encode_REJECT(const struct ovnact_nest *on,
> +              const struct ovnact_encode_params *ep,
> +              struct ofpbuf *ofpacts)
> +{
> +    encode_nested_actions(on, ep, ACTION_OPCODE_REJECT, ofpacts);
> +}
> +
>  static void
>  encode_ND_NA(const struct ovnact_nest *on,
>               const struct ovnact_encode_params *ep,
> @@ -3567,6 +3587,8 @@ parse_action(struct action_context *ctx)
>          parse_fwd_group_action(ctx);
>      } else if (lexer_match_id(ctx->lexer, "handle_dhcpv6_reply")) {
>          ovnact_put_DHCP6_REPLY(ctx->ovnacts);
> +    } else if (lexer_match_id(ctx->lexer, "reject")) {
> +        parse_REJECT(ctx);
>      } else {
>          lexer_syntax_error(ctx->lexer, "expecting action");
>      }
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 59888a1553..2fc84f54ee 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2191,6 +2191,16 @@ tcp.flags = RST;
>            <p><b>Prerequisite:</b> <code>tcp</code></p>
>          </dd>
>  
> +        <dt><code>reject { <var>action</var>; </code>...<code> };</code></dt>
> +        <dd>
> +          <p>
> +            If the original packet is IPv4 or IPv6 TCP packet, it replaces it
> +            with IPv4 or IPv6 TCP RST packet and executes the inner actions.
> +            Otherwise it replaces it with an ICMPv4 or ICMPv6 packet and
> +            executes the inner actions.
> +          </p>
> +        </dd>
> +
>          <dt><code>trigger_event;</code></dt>
>          <dd>
>            <p>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 488fd119b1..71837a6721 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1593,6 +1593,18 @@ tcp_reset { };
>      encodes as controller(userdata=00.00.00.0b.00.00.00.00)
>      has prereqs tcp
>  
> +# 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)
> +
> +reject { eth.dst <-> eth.src; ip4.src <-> ip4.dst; output; };
> +    encodes as controller(userdata=00.00.00.16.00.00.00.00.ff.ff.00.18.00.00.23.20.00.1b.00.00.00.00.04.06.00.30.00.00.00.00.00.00.ff.ff.00.18.00.00.23.20.00.1b.00.00.00.00.02.06.00.30.00.00.00.00.00.00.ff.ff.00.18.00.00.23.20.00.1c.00.00.00.00.04.06.00.30.00.00.00.00.00.00.ff.ff.00.18.00.00.23.20.00.1c.00.00.00.00.02.06.00.30.00.00.00.00.00.00.ff.ff.00.18.00.00.23.20.00.1b.00.00.00.00.10.04.00.20.00.00.00.00.00.00.ff.ff.00.18.00.00.23.20.00.1b.00.00.00.00.0e.04.00.20.00.00.00.00.00.00.ff.ff.00.18.00.00.23.20.00.1c.00.00.00.00.10.04.00.20.00.00.00.00.00.00.ff.ff.00.18.00.00.23.20.00.1c.00.00.00.00.0e.04.00.20.00.00.00.00.00.00.ff.ff.00.10.00.00.23.20.00.0e.ff.f8.40.00.00.00)
> +    has prereqs eth.type == 0x800 && eth.type == 0x800
> +
> +reject { };
> +    formats as reject { drop; };
> +    encodes as controller(userdata=00.00.00.16.00.00.00.00)
> +
>  # trigger_event
>  trigger_event(event = "empty_lb_backends", vip = "10.0.0.1:80", protocol = "tcp", load_balancer = "12345678-abcd-9876-fedc-11119f8e7d6c");
>      encodes as controller(userdata=00.00.00.0f.00.00.00.00.00.00.00.00.00.01.00.0b.31.30.2e.30.2e.30.2e.31.3a.38.30.00.02.00.03.74.63.70.00.03.00.24.31.32.33.34.35.36.37.38.2d.61.62.63.64.2d.39.38.37.36.2d.66.65.64.63.2d.31.31.31.31.39.66.38.65.37.64.36.63)
> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c
> index 0920ae1599..38aee6081b 100644
> --- a/utilities/ovn-trace.c
> +++ b/utilities/ovn-trace.c
> @@ -1711,6 +1711,23 @@ execute_tcp_reset(const struct ovnact_nest *on,
>                    table_id, pipeline, &node->subs);
>  }
>  
> +static void
> +execute_reject(const struct ovnact_nest *on,
> +               const struct ovntrace_datapath *dp,
> +               const struct flow *uflow, uint8_t table_id,
> +               enum ovnact_pipeline pipeline, struct ovs_list *super)
> +{
> +    if (uflow->nw_proto == IPPROTO_TCP) {
> +        execute_tcp_reset(on, dp, uflow, table_id, pipeline, super);

Hi Numan,

I just noticed that execute_tcp_reset() always assume IPv4 packets.  We should
probably fix it at some point to make it work with IPv6 too.  However, this is
not related to your patch.

Acked-by: Dumitru Ceara <dceara at redhat.com>

Thanks,
Dumitru

> +    } else {
> +        if (get_dl_type(uflow) == htons(ETH_TYPE_IP)) {
> +            execute_icmp4(on, dp, uflow, table_id, pipeline, super);
> +        } else {
> +            execute_icmp6(on, dp, uflow, table_id, pipeline, super);
> +        }
> +    }
> +}
> +
>  static void
>  execute_get_mac_bind(const struct ovnact_get_mac_bind *bind,
>                       const struct ovntrace_datapath *dp,
> @@ -2347,6 +2364,11 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
>              execute_ovnfield_load(ovnact_get_OVNFIELD_LOAD(a), super);
>              break;
>  
> +        case OVNACT_REJECT:
> +            execute_reject(ovnact_get_REJECT(a), dp, uflow, table_id,
> +                           pipeline, super);
> +            break;
> +
>          case OVNACT_TRIGGER_EVENT:
>              break;
>  
> 



More information about the dev mailing list