Re: [ovs-dev] [PATCH v28 4/8] netdev-offload-tc: Add sample offload API for TC

2024-03-25 Thread Chris Mi via dev

On 6/24/2023 4:17 AM, Ilya Maximets wrote:

On 6/19/23 07:05, Chris Mi wrote:

Initialize psample socket. Add sample recv API to receive sampled
packets from psample socket. Add sample recv wait API to add psample
socket fd to poll list.

Signed-off-by: Chris Mi 
Reviewed-by: Roi Dayan 
---
  lib/dpif.h|   6 +-
  lib/flow.h|   2 +-
  lib/netdev-offload-provider.h |  30 ++
  lib/netdev-offload-tc.c   | 172 ++
  lib/netdev-offload.c  |   3 +-
  lib/packets.h |   2 +-
  6 files changed, 210 insertions(+), 5 deletions(-)

diff --git a/lib/dpif.h b/lib/dpif.h
index 129cbf6a1..f91295862 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -834,8 +834,10 @@ struct dpif_upcall {
  
  /* DPIF_UC_ACTION only. */

  struct nlattr *userdata;/* Argument to OVS_ACTION_ATTR_USERSPACE. */
-struct nlattr *out_tun_key;/* Output tunnel key. */
-struct nlattr *actions;/* Argument to OVS_ACTION_ATTR_USERSPACE. */
+struct nlattr *out_tun_key; /* Output tunnel key. */
+struct nlattr *actions; /* Argument to OVS_ACTION_ATTR_USERSPACE. */
+struct flow flow;   /* Caller provided 'flow' if the 'key' is not
+   available. */
  };
  
  /* A callback to notify higher layer of dpif about to be purged, so that

diff --git a/lib/flow.h b/lib/flow.h
index a9d026e1c..0974bfd42 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -970,7 +970,7 @@ pkt_metadata_from_flow(struct pkt_metadata *md, const 
struct flow *flow)
  
  md->recirc_id = flow->recirc_id;

  md->dp_hash = flow->dp_hash;
-flow_tnl_copy__(>tunnel, >tunnel);
+flow_tnl_copy(>tunnel, >tunnel);
  md->skb_priority = flow->skb_priority;
  md->pkt_mark = flow->pkt_mark;
  md->in_port = flow->in_port;
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 9108856d1..a457556e5 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -28,6 +28,8 @@
  extern "C" {
  #endif
  
+struct dpif_upcall;

+
  struct netdev_flow_api {
  char *type;
  /* Flush all offloaded flows from a netdev.
@@ -121,6 +123,34 @@ struct netdev_flow_api {
  int (*meter_del)(ofproto_meter_id meter_id,
   struct ofputil_meter_stats *stats);
  
+/* Polls for upcall offload packets for an upcall handler. If successful,

+ * stores the upcall into '*upcall', using 'buf' for storage.
+ *
+ * The implementation should point 'upcall->flow' and 'upcall->userdata'
+ * (if any) into data in the caller-provided 'buf'.  The implementation may


This is not really correct.  The 'flow' is not a pointer in this patch set,
AFAICT, so we can't point it to buf.


I changed to '>flow'.




+ * also use 'buf' for storing the data of 'upcall->packet'.  If necessary
+ * to make room, the implementation may reallocate the data in 'buf'.
+ *
+ * The caller owns the data of 'upcall->packet' and may modify it.  If
+ * packet's headroom is exhausted as it is manipulated, 'upcall->packet'
+ * will be reallocated.  This requires the data of 'upcall->packet' to be
+ * released with ofpbuf_uninit() before 'upcall' is destroyed.  However,
+ * when an error is returned, the 'upcall->packet' may be uninitialized
+ * and should not be released.
+ *
+ * This function must not block. If no upcall is pending when it is
+ * called, it should return EAGAIN without blocking.
+ *
+ * Return 0 if successful, otherwise returns a positive errno value.
+ */
+int (*recv)(struct dpif_upcall *upcall, struct ofpbuf *buf,
+uint32_t handler_id);
+
+/* Arranges for the poll loop for an upcall handler to wake up when
+ * sample socket has a message queued to be received with the recv


s/sample socket/offload provider/


Done.




+ * member functions. */
+void (*recv_wait)(uint32_t handler_id);
+
  /* Initializies the netdev flow api.
   * Return 0 if successful, otherwise returns a positive errno value. */
  int (*init_flow_api)(struct netdev *);
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 56726acf8..8d571aca8 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -18,6 +18,8 @@
  
  #include 

  #include 
+#include 
+#include 
  
  #include "cmap.h"

  #include "dpif-provider.h"
@@ -125,6 +127,9 @@ struct sgid_node {
  struct offload_sample sample;
  };
  
+static struct nl_sock *psample_sock;

+static int psample_family;
+
  /* The sgid_map mutex protects the sample_group_ids and the sgid_map for
   * cmap_insert(), cmap_remove(), or cmap_replace() operations. */
  static struct ovs_mutex sgid_lock = OVS_MUTEX_INITIALIZER;
@@ -156,6 +161,14 @@ sgid_find(uint32_t id)
  return node ? CONTAINER_OF(node, struct sgid_node, id_node) : NULL;
  }
  
+static struct offload_sample *

+sample_find(uint32_t id)
+{
+

Re: [ovs-dev] [PATCH v28 4/8] netdev-offload-tc: Add sample offload API for TC

2023-06-23 Thread Ilya Maximets
On 6/19/23 07:05, Chris Mi wrote:
> Initialize psample socket. Add sample recv API to receive sampled
> packets from psample socket. Add sample recv wait API to add psample
> socket fd to poll list.
> 
> Signed-off-by: Chris Mi 
> Reviewed-by: Roi Dayan 
> ---
>  lib/dpif.h|   6 +-
>  lib/flow.h|   2 +-
>  lib/netdev-offload-provider.h |  30 ++
>  lib/netdev-offload-tc.c   | 172 ++
>  lib/netdev-offload.c  |   3 +-
>  lib/packets.h |   2 +-
>  6 files changed, 210 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/dpif.h b/lib/dpif.h
> index 129cbf6a1..f91295862 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -834,8 +834,10 @@ struct dpif_upcall {
>  
>  /* DPIF_UC_ACTION only. */
>  struct nlattr *userdata;/* Argument to OVS_ACTION_ATTR_USERSPACE. */
> -struct nlattr *out_tun_key;/* Output tunnel key. */
> -struct nlattr *actions;/* Argument to OVS_ACTION_ATTR_USERSPACE. */
> +struct nlattr *out_tun_key; /* Output tunnel key. */
> +struct nlattr *actions; /* Argument to OVS_ACTION_ATTR_USERSPACE. */
> +struct flow flow;   /* Caller provided 'flow' if the 'key' is not
> +   available. */
>  };
>  
>  /* A callback to notify higher layer of dpif about to be purged, so that
> diff --git a/lib/flow.h b/lib/flow.h
> index a9d026e1c..0974bfd42 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -970,7 +970,7 @@ pkt_metadata_from_flow(struct pkt_metadata *md, const 
> struct flow *flow)
>  
>  md->recirc_id = flow->recirc_id;
>  md->dp_hash = flow->dp_hash;
> -flow_tnl_copy__(>tunnel, >tunnel);
> +flow_tnl_copy(>tunnel, >tunnel);
>  md->skb_priority = flow->skb_priority;
>  md->pkt_mark = flow->pkt_mark;
>  md->in_port = flow->in_port;
> diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
> index 9108856d1..a457556e5 100644
> --- a/lib/netdev-offload-provider.h
> +++ b/lib/netdev-offload-provider.h
> @@ -28,6 +28,8 @@
>  extern "C" {
>  #endif
>  
> +struct dpif_upcall;
> +
>  struct netdev_flow_api {
>  char *type;
>  /* Flush all offloaded flows from a netdev.
> @@ -121,6 +123,34 @@ struct netdev_flow_api {
>  int (*meter_del)(ofproto_meter_id meter_id,
>   struct ofputil_meter_stats *stats);
>  
> +/* Polls for upcall offload packets for an upcall handler. If successful,
> + * stores the upcall into '*upcall', using 'buf' for storage.
> + *
> + * The implementation should point 'upcall->flow' and 'upcall->userdata'
> + * (if any) into data in the caller-provided 'buf'.  The implementation 
> may

This is not really correct.  The 'flow' is not a pointer in this patch set,
AFAICT, so we can't point it to buf.

> + * also use 'buf' for storing the data of 'upcall->packet'.  If necessary
> + * to make room, the implementation may reallocate the data in 'buf'.
> + *
> + * The caller owns the data of 'upcall->packet' and may modify it.  If
> + * packet's headroom is exhausted as it is manipulated, 'upcall->packet'
> + * will be reallocated.  This requires the data of 'upcall->packet' to be
> + * released with ofpbuf_uninit() before 'upcall' is destroyed.  However,
> + * when an error is returned, the 'upcall->packet' may be uninitialized
> + * and should not be released.
> + *
> + * This function must not block. If no upcall is pending when it is
> + * called, it should return EAGAIN without blocking.
> + *
> + * Return 0 if successful, otherwise returns a positive errno value.
> + */
> +int (*recv)(struct dpif_upcall *upcall, struct ofpbuf *buf,
> +uint32_t handler_id);
> +
> +/* Arranges for the poll loop for an upcall handler to wake up when
> + * sample socket has a message queued to be received with the recv

s/sample socket/offload provider/

> + * member functions. */
> +void (*recv_wait)(uint32_t handler_id);
> +
>  /* Initializies the netdev flow api.
>   * Return 0 if successful, otherwise returns a positive errno value. */
>  int (*init_flow_api)(struct netdev *);
> diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
> index 56726acf8..8d571aca8 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -18,6 +18,8 @@
>  
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #include "cmap.h"
>  #include "dpif-provider.h"
> @@ -125,6 +127,9 @@ struct sgid_node {
>  struct offload_sample sample;
>  };
>  
> +static struct nl_sock *psample_sock;
> +static int psample_family;
> +
>  /* The sgid_map mutex protects the sample_group_ids and the sgid_map for
>   * cmap_insert(), cmap_remove(), or cmap_replace() operations. */
>  static struct ovs_mutex sgid_lock = OVS_MUTEX_INITIALIZER;
> @@ -156,6 +161,14 @@ sgid_find(uint32_t id)
>  return node ? CONTAINER_OF(node, struct 

Re: [ovs-dev] [PATCH v28 4/8] netdev-offload-tc: Add sample offload API for TC

2023-06-19 Thread Eelco Chaudron



On 19 Jun 2023, at 7:05, Chris Mi wrote:

> Initialize psample socket. Add sample recv API to receive sampled
> packets from psample socket. Add sample recv wait API to add psample
> socket fd to poll list.
>
> Signed-off-by: Chris Mi 
> Reviewed-by: Roi Dayan 

You forgot to include my ACK on v27. So here it is again:

Acked-by: Eelco Chaudron 

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


[ovs-dev] [PATCH v28 4/8] netdev-offload-tc: Add sample offload API for TC

2023-06-18 Thread Chris Mi via dev
Initialize psample socket. Add sample recv API to receive sampled
packets from psample socket. Add sample recv wait API to add psample
socket fd to poll list.

Signed-off-by: Chris Mi 
Reviewed-by: Roi Dayan 
---
 lib/dpif.h|   6 +-
 lib/flow.h|   2 +-
 lib/netdev-offload-provider.h |  30 ++
 lib/netdev-offload-tc.c   | 172 ++
 lib/netdev-offload.c  |   3 +-
 lib/packets.h |   2 +-
 6 files changed, 210 insertions(+), 5 deletions(-)

diff --git a/lib/dpif.h b/lib/dpif.h
index 129cbf6a1..f91295862 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -834,8 +834,10 @@ struct dpif_upcall {
 
 /* DPIF_UC_ACTION only. */
 struct nlattr *userdata;/* Argument to OVS_ACTION_ATTR_USERSPACE. */
-struct nlattr *out_tun_key;/* Output tunnel key. */
-struct nlattr *actions;/* Argument to OVS_ACTION_ATTR_USERSPACE. */
+struct nlattr *out_tun_key; /* Output tunnel key. */
+struct nlattr *actions; /* Argument to OVS_ACTION_ATTR_USERSPACE. */
+struct flow flow;   /* Caller provided 'flow' if the 'key' is not
+   available. */
 };
 
 /* A callback to notify higher layer of dpif about to be purged, so that
diff --git a/lib/flow.h b/lib/flow.h
index a9d026e1c..0974bfd42 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -970,7 +970,7 @@ pkt_metadata_from_flow(struct pkt_metadata *md, const 
struct flow *flow)
 
 md->recirc_id = flow->recirc_id;
 md->dp_hash = flow->dp_hash;
-flow_tnl_copy__(>tunnel, >tunnel);
+flow_tnl_copy(>tunnel, >tunnel);
 md->skb_priority = flow->skb_priority;
 md->pkt_mark = flow->pkt_mark;
 md->in_port = flow->in_port;
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 9108856d1..a457556e5 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -28,6 +28,8 @@
 extern "C" {
 #endif
 
+struct dpif_upcall;
+
 struct netdev_flow_api {
 char *type;
 /* Flush all offloaded flows from a netdev.
@@ -121,6 +123,34 @@ struct netdev_flow_api {
 int (*meter_del)(ofproto_meter_id meter_id,
  struct ofputil_meter_stats *stats);
 
+/* Polls for upcall offload packets for an upcall handler. If successful,
+ * stores the upcall into '*upcall', using 'buf' for storage.
+ *
+ * The implementation should point 'upcall->flow' and 'upcall->userdata'
+ * (if any) into data in the caller-provided 'buf'.  The implementation may
+ * also use 'buf' for storing the data of 'upcall->packet'.  If necessary
+ * to make room, the implementation may reallocate the data in 'buf'.
+ *
+ * The caller owns the data of 'upcall->packet' and may modify it.  If
+ * packet's headroom is exhausted as it is manipulated, 'upcall->packet'
+ * will be reallocated.  This requires the data of 'upcall->packet' to be
+ * released with ofpbuf_uninit() before 'upcall' is destroyed.  However,
+ * when an error is returned, the 'upcall->packet' may be uninitialized
+ * and should not be released.
+ *
+ * This function must not block. If no upcall is pending when it is
+ * called, it should return EAGAIN without blocking.
+ *
+ * Return 0 if successful, otherwise returns a positive errno value.
+ */
+int (*recv)(struct dpif_upcall *upcall, struct ofpbuf *buf,
+uint32_t handler_id);
+
+/* Arranges for the poll loop for an upcall handler to wake up when
+ * sample socket has a message queued to be received with the recv
+ * member functions. */
+void (*recv_wait)(uint32_t handler_id);
+
 /* Initializies the netdev flow api.
  * Return 0 if successful, otherwise returns a positive errno value. */
 int (*init_flow_api)(struct netdev *);
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 56726acf8..8d571aca8 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -18,6 +18,8 @@
 
 #include 
 #include 
+#include 
+#include 
 
 #include "cmap.h"
 #include "dpif-provider.h"
@@ -125,6 +127,9 @@ struct sgid_node {
 struct offload_sample sample;
 };
 
+static struct nl_sock *psample_sock;
+static int psample_family;
+
 /* The sgid_map mutex protects the sample_group_ids and the sgid_map for
  * cmap_insert(), cmap_remove(), or cmap_replace() operations. */
 static struct ovs_mutex sgid_lock = OVS_MUTEX_INITIALIZER;
@@ -156,6 +161,14 @@ sgid_find(uint32_t id)
 return node ? CONTAINER_OF(node, struct sgid_node, id_node) : NULL;
 }
 
+static struct offload_sample *
+sample_find(uint32_t id)
+{
+struct sgid_node *node = sgid_find(id);
+
+return node ? >sample: NULL;
+}
+
 static void
 offload_sample_clone(struct offload_sample *dst,
  const struct offload_sample *src,
@@ -2955,6 +2968,55 @@ tc_cleanup_policer_actions(struct id_pool *police_ids,
 hmap_destroy();
 }
 
+static void