On Fri, Jun 30, 2023 at 11:47:04AM +0200, Simon Horman wrote:

On Thu, Jun 29, 2023 at 04:30:05PM -0400, Eric Garver wrote:

quoted

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. We
can then use perf tracing to match on the drop reason.

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: 196610)

reason: 196610 --> 0x30002 (OVS_XLATE_RECURSION_TOO_DEEP)

Signed-off-by: Eric Garver <redacted>

...

quoted

--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c

@@ -32,6 +32,7 @@
 #include "vport.h"
 #include "flow_netlink.h"
 #include "openvswitch_trace.h"
+#include "drop.h"
 
 struct deferred_action {
 	struct sk_buff *skb;

@@ -1477,6 +1478,18 @@ 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:
+			u32 reason = nla_get_u32(a);
+
+			reason |= SKB_DROP_REASON_SUBSYS_OPENVSWITCH <<
+					SKB_DROP_REASON_SUBSYS_SHIFT;
+
+			if (reason == OVS_XLATE_OK)
+				break;
+
+			kfree_skb_reason(skb, reason);
+			return 0;
 		}

Hi Eric,

thanks for your patches. This is an interesting new feature.

unfortunately clang-16 doesn't seem to like this very much.
I think that it is due to the declaration of reason not
being enclosed in a block - { }.

  net/openvswitch/actions.c:1483:4: error: expected expression
                          u32 reason = nla_get_u32(a);
                          ^
  net/openvswitch/actions.c:1485:4: error: use of undeclared identifier 'reason'
                          reason |= SKB_DROP_REASON_SUBSYS_OPENVSWITCH <<
                          ^
  net/openvswitch/actions.c:1488:8: error: use of undeclared identifier 'reason'
                          if (reason == OVS_XLATE_OK)
                              ^
  net/openvswitch/actions.c:1491:26: error: use of undeclared identifier 'reason'
                          kfree_skb_reason(skb, reason);
                                                ^
  4 errors generated.


net-next is currently closed.
So please provide a v2 once it reposts, after 10th July.

oof. My bad. I'll fix the clang issue and post v2 in a couple weeks.

Thanks.
Eric.