Adrian Moreno <amore...@redhat.com> writes:

> From: Eric Garver <e...@garver.life>
>
> From: Eric Garver <e...@garver.life>
>
> This adds an explicit drop action. This is used by OVS to drop packets
> for which it cannot determine what to do. An explicit action in the
> kernel allows passing the reason _why_ the packet is being dropped or
> zero to indicate no particular error happened (i.e: OVS intentionally
> dropped the packet).
>
> Since the error codes coming from userspace mean nothing for the kernel,
> we squash all of them into only two drop reasons:
> - OVS_DROP_EXPLICIT_ACTION_ERROR to indicate a non-zero value was passed
> - OVS_DROP_EXPLICIT_ACTION to indicate a zero value was passed (no
>   error)
>
> e.g. trace all OVS dropped skbs
>
>  # perf trace -e skb:kfree_skb --filter="reason >= 0x30000"
>  [..]
>  106.023 ping/2465 skb:kfree_skb(skbaddr: 0xffffa0e8765f2000, \
>   location:0xffffffffc0d9b462, protocol: 2048, reason: 196611)
>
> reason: 196611 --> 0x30003 (OVS_DROP_EXPLICIT_ACTION)
>
> Signed-off-by: Eric Garver <e...@garver.life>
> Co-developed-by: Adrian Moreno <amore...@redhat.com>
> Signed-off-by: Adrian Moreno <amore...@redhat.com>
> ---
>  include/uapi/linux/openvswitch.h                     |  2 ++
>  net/openvswitch/actions.c                            |  9 +++++++++
>  net/openvswitch/drop.h                               |  2 ++
>  net/openvswitch/flow_netlink.c                       | 10 +++++++++-
>  tools/testing/selftests/net/openvswitch/ovs-dpctl.py |  3 +++
>  5 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index e94870e77ee9..efc82c318fa2 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -965,6 +965,7 @@ struct check_pkt_len_arg {
>   * start of the packet or at the start of the l3 header depending on the 
> value
>   * of l3 tunnel flag in the tun_flags field of OVS_ACTION_ATTR_ADD_MPLS
>   * argument.
> + * @OVS_ACTION_ATTR_DROP: Explicit drop action.
>   *
>   * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not 
> all
>   * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
> @@ -1002,6 +1003,7 @@ enum ovs_action_attr {
>       OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */
>       OVS_ACTION_ATTR_ADD_MPLS,     /* struct ovs_action_add_mpls. */
>       OVS_ACTION_ATTR_DEC_TTL,      /* Nested OVS_DEC_TTL_ATTR_*. */
> +     OVS_ACTION_ATTR_DROP,         /* u32 error code. */
>  
>       __OVS_ACTION_ATTR_MAX,        /* Nothing past this will be accepted
>                                      * from userspace. */
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 9b66a3334aaa..285b1243b94f 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -1485,6 +1485,15 @@ static int do_execute_actions(struct datapath *dp, 
> struct sk_buff *skb,
>                               return dec_ttl_exception_handler(dp, skb,
>                                                                key, a);
>                       break;
> +
> +             case OVS_ACTION_ATTR_DROP: {
> +                     enum ovs_drop_reason reason = nla_get_u32(a)
> +                             ? OVS_DROP_EXPLICIT_ACTION_ERROR
> +                             : OVS_DROP_EXPLICIT_ACTION;
> +
> +                     kfree_skb_reason(skb, reason);
> +                     return 0;
> +             }
>               }
>  
>               if (unlikely(err)) {
> diff --git a/net/openvswitch/drop.h b/net/openvswitch/drop.h
> index 3cd6489a5a2b..be51ff5039fb 100644
> --- a/net/openvswitch/drop.h
> +++ b/net/openvswitch/drop.h
> @@ -10,6 +10,8 @@
>  #define OVS_DROP_REASONS(R)                  \
>       R(OVS_DROP_FLOW)                        \
>       R(OVS_DROP_ACTION_ERROR)                \
> +     R(OVS_DROP_EXPLICIT_ACTION)             \
> +     R(OVS_DROP_EXPLICIT_ACTION_ERROR)       \
>       /* deliberate comment for trailing \ */
>  
>  enum ovs_drop_reason {
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index 41116361433d..88965e2068ac 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -38,6 +38,7 @@
>  #include <net/tun_proto.h>
>  #include <net/erspan.h>
>  
> +#include "drop.h"
>  #include "flow_netlink.h"
>  
>  struct ovs_len_tbl {
> @@ -61,6 +62,7 @@ static bool actions_may_change_flow(const struct nlattr 
> *actions)
>               case OVS_ACTION_ATTR_RECIRC:
>               case OVS_ACTION_ATTR_TRUNC:
>               case OVS_ACTION_ATTR_USERSPACE:
> +             case OVS_ACTION_ATTR_DROP:
>                       break;
>  
>               case OVS_ACTION_ATTR_CT:
> @@ -2394,7 +2396,7 @@ static void ovs_nla_free_nested_actions(const struct 
> nlattr *actions, int len)
>       /* Whenever new actions are added, the need to update this
>        * function should be considered.
>        */
> -     BUILD_BUG_ON(OVS_ACTION_ATTR_MAX != 23);
> +     BUILD_BUG_ON(OVS_ACTION_ATTR_MAX != 24);
>  
>       if (!actions)
>               return;
> @@ -3182,6 +3184,7 @@ static int __ovs_nla_copy_actions(struct net *net, 
> const struct nlattr *attr,
>                       [OVS_ACTION_ATTR_CHECK_PKT_LEN] = (u32)-1,
>                       [OVS_ACTION_ATTR_ADD_MPLS] = sizeof(struct 
> ovs_action_add_mpls),
>                       [OVS_ACTION_ATTR_DEC_TTL] = (u32)-1,
> +                     [OVS_ACTION_ATTR_DROP] = sizeof(u32),
>               };
>               const struct ovs_action_push_vlan *vlan;
>               int type = nla_type(a);
> @@ -3453,6 +3456,11 @@ static int __ovs_nla_copy_actions(struct net *net, 
> const struct nlattr *attr,
>                       skip_copy = true;
>                       break;
>  
> +             case OVS_ACTION_ATTR_DROP:
> +                     if (!nla_is_last(a, rem))
> +                             return -EINVAL;
> +                     break;
> +
>               default:
>                       OVS_NLERR(log, "Unknown Action type %d", type);
>                       return -EINVAL;
> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py 
> b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> index fbdac15e3134..5fee330050c2 100644
> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> @@ -301,6 +301,7 @@ class ovsactions(nla):
>          ("OVS_ACTION_ATTR_CHECK_PKT_LEN", "none"),
>          ("OVS_ACTION_ATTR_ADD_MPLS", "none"),
>          ("OVS_ACTION_ATTR_DEC_TTL", "none"),
> +        ("OVS_ACTION_ATTR_DROP", "uint32"),
>      )
>  
>      class ctact(nla):
> @@ -447,6 +448,8 @@ class ovsactions(nla):
>                      print_str += "recirc(0x%x)" % 
> int(self.get_attr(field[0]))
>                  elif field[0] == "OVS_ACTION_ATTR_TRUNC":
>                      print_str += "trunc(%d)" % int(self.get_attr(field[0]))
> +                elif field[0] == "OVS_ACTION_ATTR_DROP":
> +                    print_str += "drop"

Any reason that you don't include the int(self.get_attr(field[0])) here
and add it only to 7/7?

>              elif field[1] == "flag":
>                  if field[0] == "OVS_ACTION_ATTR_CT_CLEAR":
>                      print_str += "ct_clear"

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to