Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 3/6/2023 4:51 PM, Eelco Chaudron wrote:
On 6 Mar 2023, at 9:19, Chris Mi wrote:
On 3/1/2023 9:23 PM, Ilya Maximets wrote:
On 3/1/23 14:21, Ilya Maximets wrote:
On 3/1/23 14:08, Chris Mi wrote:
On 3/1/2023 8:44 PM, Ilya Maximets wrote:
On 2/28/23 14:05, Chris Mi wrote:
On 2/24/2023 4:16 AM, Ilya Maximets wrote:
On 2/23/23 12:26, Chris Mi wrote:
Initialize psample socket. Add sFlow recv API to receive sampled
packets from psample socket. Add sFow recv wait API to add psample
socket fd to poll list.
Signed-off-by: Chris Mi
Reviewed-by: Roi Dayan
---
lib/dpif.h| 7 ++
lib/netdev-offload-provider.h | 11 ++
lib/netdev-offload-tc.c | 188 ++
3 files changed, 206 insertions(+)
+{
+int read_tries = 0;
+
+if (!psample_sock) {
+return ENOENT;
+}
+
+for (;;) {
+struct offload_psample psample;
+struct offload_sflow sflow;
+int error;
+
+if (++read_tries > 50) {
+return EAGAIN;
+}
+
+error = nl_sock_recv(psample_sock, buf, NULL, false);
+if (error == ENOBUFS) {
+continue;
+}
+
+if (error) {
+if (error == EAGAIN) {
+break;
+}
+return error;
+}
+
+error = psample_from_ofpbuf(, buf);
+if (!error) {
+return psample_parse_packet(, , upcall);
+} else if (error) {
Condition here is always true.
I copied from dpif_netlink_recv_cpu_dispatch(). And I also think it is ok.
The 'if' condition in dpif_netlink_recv_cpu_dispatch() is different
and so the 'else if (error)' is not always true there. But it is
always true here, hence makes no practical sense.
OK, I see. I'll change the code like this:
error = psample_from_ofpbuf(, buf);
if (!error) {
return psample_parse_packet(, upcall);
} else {
return error;
}
The 'else' condition is unnecessary, since we do always return
at this point. In the dpif-netlink code there is a case where
we continue, but here there is no such case.
So, I'd suggest:
error = psample_from_ofpbuf(, buf);
if (error) {
return error;
}
return psample_parse_packet(, upcall);
Or even just a ternary operator:
error = psample_from_ofpbuf(, buf);
return error ? error : psample_parse_packet(, upcall);
Just in case, please wait for a proper review on v24 from someone
before sending a new version. There is no point to re-spin the
set just for this.
OK.
Eelco,
You reviewed my previous versions and acked most of them. But since Ilya
suggested
a new design, the code was changed greatly. So I removed the Acked-by for you.
I'm wondering
if you have time to review again recently?
Yes, it’s on my todo for this week, but might slip into next week…
//Eelco
No problem.
Thanks,
Chris
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 6 Mar 2023, at 9:19, Chris Mi wrote:
> On 3/1/2023 9:23 PM, Ilya Maximets wrote:
>> On 3/1/23 14:21, Ilya Maximets wrote:
>>> On 3/1/23 14:08, Chris Mi wrote:
On 3/1/2023 8:44 PM, Ilya Maximets wrote:
> On 2/28/23 14:05, Chris Mi wrote:
>> On 2/24/2023 4:16 AM, Ilya Maximets wrote:
>>> On 2/23/23 12:26, Chris Mi wrote:
Initialize psample socket. Add sFlow recv API to receive sampled
packets from psample socket. Add sFow recv wait API to add psample
socket fd to poll list.
Signed-off-by: Chris Mi
Reviewed-by: Roi Dayan
---
lib/dpif.h| 7 ++
lib/netdev-offload-provider.h | 11 ++
lib/netdev-offload-tc.c | 188
++
3 files changed, 206 insertions(+)
>
>
+{
+int read_tries = 0;
+
+if (!psample_sock) {
+return ENOENT;
+}
+
+for (;;) {
+struct offload_psample psample;
+struct offload_sflow sflow;
+int error;
+
+if (++read_tries > 50) {
+return EAGAIN;
+}
+
+error = nl_sock_recv(psample_sock, buf, NULL, false);
+if (error == ENOBUFS) {
+continue;
+}
+
+if (error) {
+if (error == EAGAIN) {
+break;
+}
+return error;
+}
+
+error = psample_from_ofpbuf(, buf);
+if (!error) {
+return psample_parse_packet(, , upcall);
+} else if (error) {
>>> Condition here is always true.
>> I copied from dpif_netlink_recv_cpu_dispatch(). And I also think it is
>> ok.
> The 'if' condition in dpif_netlink_recv_cpu_dispatch() is different
> and so the 'else if (error)' is not always true there. But it is
> always true here, hence makes no practical sense.
OK, I see. I'll change the code like this:
error = psample_from_ofpbuf(, buf);
if (!error) {
return psample_parse_packet(, upcall);
} else {
return error;
}
>>> The 'else' condition is unnecessary, since we do always return
>>> at this point. In the dpif-netlink code there is a case where
>>> we continue, but here there is no such case.
>>>
>>> So, I'd suggest:
>>>
>>> error = psample_from_ofpbuf(, buf);
>>> if (error) {
>>> return error;
>>> }
>>>
>>> return psample_parse_packet(, upcall);
>>>
>>> Or even just a ternary operator:
>>>
>>> error = psample_from_ofpbuf(, buf);
>>>
>>> return error ? error : psample_parse_packet(, upcall);
>> Just in case, please wait for a proper review on v24 from someone
>> before sending a new version. There is no point to re-spin the
>> set just for this.
> OK.
>
> Eelco,
>
> You reviewed my previous versions and acked most of them. But since Ilya
> suggested
> a new design, the code was changed greatly. So I removed the Acked-by for
> you. I'm wondering
> if you have time to review again recently?
Yes, it’s on my todo for this week, but might slip into next week…
//Eelco
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 3/1/2023 9:23 PM, Ilya Maximets wrote:
On 3/1/23 14:21, Ilya Maximets wrote:
On 3/1/23 14:08, Chris Mi wrote:
On 3/1/2023 8:44 PM, Ilya Maximets wrote:
On 2/28/23 14:05, Chris Mi wrote:
On 2/24/2023 4:16 AM, Ilya Maximets wrote:
On 2/23/23 12:26, Chris Mi wrote:
Initialize psample socket. Add sFlow recv API to receive sampled
packets from psample socket. Add sFow recv wait API to add psample
socket fd to poll list.
Signed-off-by: Chris Mi
Reviewed-by: Roi Dayan
---
lib/dpif.h| 7 ++
lib/netdev-offload-provider.h | 11 ++
lib/netdev-offload-tc.c | 188 ++
3 files changed, 206 insertions(+)
+{
+int read_tries = 0;
+
+if (!psample_sock) {
+return ENOENT;
+}
+
+for (;;) {
+struct offload_psample psample;
+struct offload_sflow sflow;
+int error;
+
+if (++read_tries > 50) {
+return EAGAIN;
+}
+
+error = nl_sock_recv(psample_sock, buf, NULL, false);
+if (error == ENOBUFS) {
+continue;
+}
+
+if (error) {
+if (error == EAGAIN) {
+break;
+}
+return error;
+}
+
+error = psample_from_ofpbuf(, buf);
+if (!error) {
+return psample_parse_packet(, , upcall);
+} else if (error) {
Condition here is always true.
I copied from dpif_netlink_recv_cpu_dispatch(). And I also think it is ok.
The 'if' condition in dpif_netlink_recv_cpu_dispatch() is different
and so the 'else if (error)' is not always true there. But it is
always true here, hence makes no practical sense.
OK, I see. I'll change the code like this:
error = psample_from_ofpbuf(, buf);
if (!error) {
return psample_parse_packet(, upcall);
} else {
return error;
}
The 'else' condition is unnecessary, since we do always return
at this point. In the dpif-netlink code there is a case where
we continue, but here there is no such case.
So, I'd suggest:
error = psample_from_ofpbuf(, buf);
if (error) {
return error;
}
return psample_parse_packet(, upcall);
Or even just a ternary operator:
error = psample_from_ofpbuf(, buf);
return error ? error : psample_parse_packet(, upcall);
Just in case, please wait for a proper review on v24 from someone
before sending a new version. There is no point to re-spin the
set just for this.
OK.
Eelco,
You reviewed my previous versions and acked most of them. But since Ilya
suggested
a new design, the code was changed greatly. So I removed the Acked-by
for you. I'm wondering
if you have time to review again recently?
Thanks,
Chris
Best regards, Ilya Maximets
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 3/1/23 14:21, Ilya Maximets wrote:
> On 3/1/23 14:08, Chris Mi wrote:
>> On 3/1/2023 8:44 PM, Ilya Maximets wrote:
>>> On 2/28/23 14:05, Chris Mi wrote:
On 2/24/2023 4:16 AM, Ilya Maximets wrote:
> On 2/23/23 12:26, Chris Mi wrote:
>> Initialize psample socket. Add sFlow recv API to receive sampled
>> packets from psample socket. Add sFow recv wait API to add psample
>> socket fd to poll list.
>>
>> Signed-off-by: Chris Mi
>> Reviewed-by: Roi Dayan
>> ---
>> lib/dpif.h| 7 ++
>> lib/netdev-offload-provider.h | 11 ++
>> lib/netdev-offload-tc.c | 188 ++
>> 3 files changed, 206 insertions(+)
>>
>>>
>>>
>> +{
>> +int read_tries = 0;
>> +
>> +if (!psample_sock) {
>> +return ENOENT;
>> +}
>> +
>> +for (;;) {
>> +struct offload_psample psample;
>> +struct offload_sflow sflow;
>> +int error;
>> +
>> +if (++read_tries > 50) {
>> +return EAGAIN;
>> +}
>> +
>> +error = nl_sock_recv(psample_sock, buf, NULL, false);
>> +if (error == ENOBUFS) {
>> +continue;
>> +}
>> +
>> +if (error) {
>> +if (error == EAGAIN) {
>> +break;
>> +}
>> +return error;
>> +}
>> +
>> +error = psample_from_ofpbuf(, buf);
>> +if (!error) {
>> +return psample_parse_packet(, , upcall);
>> +} else if (error) {
> Condition here is always true.
I copied from dpif_netlink_recv_cpu_dispatch(). And I also think it is ok.
>>> The 'if' condition in dpif_netlink_recv_cpu_dispatch() is different
>>> and so the 'else if (error)' is not always true there. But it is
>>> always true here, hence makes no practical sense.
>> OK, I see. I'll change the code like this:
>>
>> error = psample_from_ofpbuf(, buf);
>> if (!error) {
>> return psample_parse_packet(, upcall);
>> } else {
>> return error;
>> }
>
> The 'else' condition is unnecessary, since we do always return
> at this point. In the dpif-netlink code there is a case where
> we continue, but here there is no such case.
>
> So, I'd suggest:
>
> error = psample_from_ofpbuf(, buf);
> if (error) {
> return error;
> }
>
> return psample_parse_packet(, upcall);
>
> Or even just a ternary operator:
>
> error = psample_from_ofpbuf(, buf);
>
> return error ? error : psample_parse_packet(, upcall);
Just in case, please wait for a proper review on v24 from someone
before sending a new version. There is no point to re-spin the
set just for this.
>
> Best regards, Ilya Maximets
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 3/1/23 14:08, Chris Mi wrote:
> On 3/1/2023 8:44 PM, Ilya Maximets wrote:
>> On 2/28/23 14:05, Chris Mi wrote:
>>> On 2/24/2023 4:16 AM, Ilya Maximets wrote:
On 2/23/23 12:26, Chris Mi wrote:
> Initialize psample socket. Add sFlow recv API to receive sampled
> packets from psample socket. Add sFow recv wait API to add psample
> socket fd to poll list.
>
> Signed-off-by: Chris Mi
> Reviewed-by: Roi Dayan
> ---
> lib/dpif.h| 7 ++
> lib/netdev-offload-provider.h | 11 ++
> lib/netdev-offload-tc.c | 188 ++
> 3 files changed, 206 insertions(+)
>
>>
>>
> +{
> +int read_tries = 0;
> +
> +if (!psample_sock) {
> +return ENOENT;
> +}
> +
> +for (;;) {
> +struct offload_psample psample;
> +struct offload_sflow sflow;
> +int error;
> +
> +if (++read_tries > 50) {
> +return EAGAIN;
> +}
> +
> +error = nl_sock_recv(psample_sock, buf, NULL, false);
> +if (error == ENOBUFS) {
> +continue;
> +}
> +
> +if (error) {
> +if (error == EAGAIN) {
> +break;
> +}
> +return error;
> +}
> +
> +error = psample_from_ofpbuf(, buf);
> +if (!error) {
> +return psample_parse_packet(, , upcall);
> +} else if (error) {
Condition here is always true.
>>> I copied from dpif_netlink_recv_cpu_dispatch(). And I also think it is ok.
>> The 'if' condition in dpif_netlink_recv_cpu_dispatch() is different
>> and so the 'else if (error)' is not always true there. But it is
>> always true here, hence makes no practical sense.
> OK, I see. I'll change the code like this:
>
> error = psample_from_ofpbuf(, buf);
> if (!error) {
> return psample_parse_packet(, upcall);
> } else {
> return error;
> }
The 'else' condition is unnecessary, since we do always return
at this point. In the dpif-netlink code there is a case where
we continue, but here there is no such case.
So, I'd suggest:
error = psample_from_ofpbuf(, buf);
if (error) {
return error;
}
return psample_parse_packet(, upcall);
Or even just a ternary operator:
error = psample_from_ofpbuf(, buf);
return error ? error : psample_parse_packet(, upcall);
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 3/1/2023 8:44 PM, Ilya Maximets wrote:
On 2/28/23 14:05, Chris Mi wrote:
On 2/24/2023 4:16 AM, Ilya Maximets wrote:
On 2/23/23 12:26, Chris Mi wrote:
Initialize psample socket. Add sFlow recv API to receive sampled
packets from psample socket. Add sFow recv wait API to add psample
socket fd to poll list.
Signed-off-by: Chris Mi
Reviewed-by: Roi Dayan
---
lib/dpif.h| 7 ++
lib/netdev-offload-provider.h | 11 ++
lib/netdev-offload-tc.c | 188 ++
3 files changed, 206 insertions(+)
+{
+int read_tries = 0;
+
+if (!psample_sock) {
+return ENOENT;
+}
+
+for (;;) {
+struct offload_psample psample;
+struct offload_sflow sflow;
+int error;
+
+if (++read_tries > 50) {
+return EAGAIN;
+}
+
+error = nl_sock_recv(psample_sock, buf, NULL, false);
+if (error == ENOBUFS) {
+continue;
+}
+
+if (error) {
+if (error == EAGAIN) {
+break;
+}
+return error;
+}
+
+error = psample_from_ofpbuf(, buf);
+if (!error) {
+return psample_parse_packet(, , upcall);
+} else if (error) {
Condition here is always true.
I copied from dpif_netlink_recv_cpu_dispatch(). And I also think it is ok.
The 'if' condition in dpif_netlink_recv_cpu_dispatch() is different
and so the 'else if (error)' is not always true there. But it is
always true here, hence makes no practical sense.
OK, I see. I'll change the code like this:
error = psample_from_ofpbuf(, buf);
if (!error) {
return psample_parse_packet(, upcall);
} else {
return error;
}
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 2/28/23 14:05, Chris Mi wrote:
> On 2/24/2023 4:16 AM, Ilya Maximets wrote:
>> On 2/23/23 12:26, Chris Mi wrote:
>>> Initialize psample socket. Add sFlow recv API to receive sampled
>>> packets from psample socket. Add sFow recv wait API to add psample
>>> socket fd to poll list.
>>>
>>> Signed-off-by: Chris Mi
>>> Reviewed-by: Roi Dayan
>>> ---
>>> lib/dpif.h| 7 ++
>>> lib/netdev-offload-provider.h | 11 ++
>>> lib/netdev-offload-tc.c | 188 ++
>>> 3 files changed, 206 insertions(+)
>>>
>>> +{
>>> +int read_tries = 0;
>>> +
>>> +if (!psample_sock) {
>>> +return ENOENT;
>>> +}
>>> +
>>> +for (;;) {
>>> +struct offload_psample psample;
>>> +struct offload_sflow sflow;
>>> +int error;
>>> +
>>> +if (++read_tries > 50) {
>>> +return EAGAIN;
>>> +}
>>> +
>>> +error = nl_sock_recv(psample_sock, buf, NULL, false);
>>> +if (error == ENOBUFS) {
>>> +continue;
>>> +}
>>> +
>>> +if (error) {
>>> +if (error == EAGAIN) {
>>> +break;
>>> +}
>>> +return error;
>>> +}
>>> +
>>> +error = psample_from_ofpbuf(, buf);
>>> +if (!error) {
>>> +return psample_parse_packet(, , upcall);
>>> +} else if (error) {
>> Condition here is always true.
> I copied from dpif_netlink_recv_cpu_dispatch(). And I also think it is ok.
The 'if' condition in dpif_netlink_recv_cpu_dispatch() is different
and so the 'else if (error)' is not always true there. But it is
always true here, hence makes no practical sense.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 2/24/2023 4:16 AM, Ilya Maximets wrote:
On 2/23/23 12:26, Chris Mi wrote:
Initialize psample socket. Add sFlow recv API to receive sampled
packets from psample socket. Add sFow recv wait API to add psample
socket fd to poll list.
Signed-off-by: Chris Mi
Reviewed-by: Roi Dayan
---
lib/dpif.h| 7 ++
lib/netdev-offload-provider.h | 11 ++
lib/netdev-offload-tc.c | 188 ++
3 files changed, 206 insertions(+)
diff --git a/lib/dpif.h b/lib/dpif.h
index 6cb4dae6d..95807af8f 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -836,6 +836,13 @@ struct dpif_upcall {
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. */
+/* SFlow offload only.
+ * When receiving sampled packets from psample socket, there is no
+ * flow key. But input tunnel and input ifindex are available. They
+ * are enough to construct flow and continue to process sFlow.
+ */
+struct flow_tnl *in_tun;/* Input tunnel key. */
+uint32_t iifindex; /* Input ifindex. */
These should be replaced with a struct flow pointer. See the comments
on patch #6.
And we don't need ofload-specific comments. We just need to allow
datapath implementations to return either key or struct flow.
Done.
};
/* A callback to notify higher layer of dpif about to be purged, so that
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 9108856d1..9e2722fd1 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -121,6 +121,17 @@ struct netdev_flow_api {
int (*meter_del)(ofproto_meter_id meter_id,
struct ofputil_meter_stats *stats);
+/* Receives sampled packets in 'buf' from psample socket and fill the
+ * necessary members in 'upcall'.
+ * Return 0 if successful, otherwise returns a positive errno value.
+ */
+int (*sflow_recv)(struct dpif_upcall *upcall, struct ofpbuf *buf);
+
+/* Add psample socket fd to poll list. Wake the upcall thread up to
+ * process it if there is any sampled packets,
+ */
+void (*sflow_recv_wait)(void);
These should be generic callbacks, not tied to psample or sflow.
There might be other reasons offload implementation wants to
send a packet to userspace. Also, psample doesn't make sense for
other offload implementations. So, just recv() and recv_wait().
Done.
+
/* 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 0dbb7954f..d7901fa68 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -18,6 +18,8 @@
#include
#include
+#include
+#include
Just , no sys.
Done.
#include "dpif.h"
#include "hash.h"
@@ -104,6 +106,9 @@ static void parse_tc_flower_to_stats(struct tc_flower
*flower,
static int get_ufid_adjust_stats(const ovs_u128 *ufid,
struct dpif_flow_stats *stats);
+static struct nl_sock *psample_sock;
+static int psample_family;
+
/* When offloading sample action to TC, userspace creates a unique ID
* to map sFlow action and tunnel info and passes this ID to kernel
* instead of the sFlow info. Psample will send this ID and sampled
@@ -151,6 +156,19 @@ sgid_find(uint32_t id)
return node ? CONTAINER_OF(node, const struct sgid_node, id_node) : NULL;
}
+static struct offload_sflow *
+sflow_find(uint32_t id)
+{
+const struct sgid_node *node;
+
+node = sgid_find(id);
+if (!node) {
+return NULL;
+}
+
+return CONST_CAST(struct offload_sflow *, >sflow);
+}
+
static uint32_t
dpif_sflow_hash(const struct offload_sflow *sflow)
{
@@ -3015,6 +3033,55 @@ tc_cleanup_policer_actions(struct id_pool *police_ids,
hmap_destroy();
}
+static void
+psample_init(void)
+{
+unsigned int psample_mcgroup;
+int err;
+
+if (!netdev_is_flow_api_enabled()) {
+VLOG_DBG("Flow API is not enabled.");
+return;
+}
+
+if (psample_sock) {
+VLOG_DBG("Psample socket is already initialized.");
+return;
+}
+
+err = nl_lookup_genl_family(PSAMPLE_GENL_NAME,
+_family);
+if (err) {
+VLOG_WARN("Generic Netlink family '%s' does not exist: %s\n"
+ "Please make sure the kernel module psample is loaded.",
+ PSAMPLE_GENL_NAME, ovs_strerror(err));
+return;
+}
+
+err = nl_lookup_genl_mcgroup(PSAMPLE_GENL_NAME,
+ PSAMPLE_NL_MCGRP_SAMPLE_NAME,
+ _mcgroup);
+if (err) {
+VLOG_WARN("Failed to join Netlink multicast group '%s': %s",
+
Re: [ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
On 2/23/23 12:26, Chris Mi wrote:
> Initialize psample socket. Add sFlow recv API to receive sampled
> packets from psample socket. Add sFow recv wait API to add psample
> socket fd to poll list.
>
> Signed-off-by: Chris Mi
> Reviewed-by: Roi Dayan
> ---
> lib/dpif.h| 7 ++
> lib/netdev-offload-provider.h | 11 ++
> lib/netdev-offload-tc.c | 188 ++
> 3 files changed, 206 insertions(+)
>
> diff --git a/lib/dpif.h b/lib/dpif.h
> index 6cb4dae6d..95807af8f 100644
> --- a/lib/dpif.h
> +++ b/lib/dpif.h
> @@ -836,6 +836,13 @@ struct dpif_upcall {
> 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. */
> +/* SFlow offload only.
> + * When receiving sampled packets from psample socket, there is no
> + * flow key. But input tunnel and input ifindex are available. They
> + * are enough to construct flow and continue to process sFlow.
> + */
> +struct flow_tnl *in_tun;/* Input tunnel key. */
> +uint32_t iifindex; /* Input ifindex. */
These should be replaced with a struct flow pointer. See the comments
on patch #6.
And we don't need ofload-specific comments. We just need to allow
datapath implementations to return either key or struct flow.
> };
>
> /* A callback to notify higher layer of dpif about to be purged, so that
> diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
> index 9108856d1..9e2722fd1 100644
> --- a/lib/netdev-offload-provider.h
> +++ b/lib/netdev-offload-provider.h
> @@ -121,6 +121,17 @@ struct netdev_flow_api {
> int (*meter_del)(ofproto_meter_id meter_id,
> struct ofputil_meter_stats *stats);
>
> +/* Receives sampled packets in 'buf' from psample socket and fill the
> + * necessary members in 'upcall'.
> + * Return 0 if successful, otherwise returns a positive errno value.
> + */
> +int (*sflow_recv)(struct dpif_upcall *upcall, struct ofpbuf *buf);
> +
> +/* Add psample socket fd to poll list. Wake the upcall thread up to
> + * process it if there is any sampled packets,
> + */
> +void (*sflow_recv_wait)(void);
These should be generic callbacks, not tied to psample or sflow.
There might be other reasons offload implementation wants to
send a packet to userspace. Also, psample doesn't make sense for
other offload implementations. So, just recv() and recv_wait().
> +
> /* 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 0dbb7954f..d7901fa68 100644
> --- a/lib/netdev-offload-tc.c
> +++ b/lib/netdev-offload-tc.c
> @@ -18,6 +18,8 @@
>
> #include
> #include
> +#include
> +#include
Just , no sys.
>
> #include "dpif.h"
> #include "hash.h"
> @@ -104,6 +106,9 @@ static void parse_tc_flower_to_stats(struct tc_flower
> *flower,
> static int get_ufid_adjust_stats(const ovs_u128 *ufid,
> struct dpif_flow_stats *stats);
>
> +static struct nl_sock *psample_sock;
> +static int psample_family;
> +
> /* When offloading sample action to TC, userspace creates a unique ID
> * to map sFlow action and tunnel info and passes this ID to kernel
> * instead of the sFlow info. Psample will send this ID and sampled
> @@ -151,6 +156,19 @@ sgid_find(uint32_t id)
> return node ? CONTAINER_OF(node, const struct sgid_node, id_node) : NULL;
> }
>
> +static struct offload_sflow *
> +sflow_find(uint32_t id)
> +{
> +const struct sgid_node *node;
> +
> +node = sgid_find(id);
> +if (!node) {
> +return NULL;
> +}
> +
> +return CONST_CAST(struct offload_sflow *, >sflow);
> +}
> +
> static uint32_t
> dpif_sflow_hash(const struct offload_sflow *sflow)
> {
> @@ -3015,6 +3033,55 @@ tc_cleanup_policer_actions(struct id_pool *police_ids,
> hmap_destroy();
> }
>
> +static void
> +psample_init(void)
> +{
> +unsigned int psample_mcgroup;
> +int err;
> +
> +if (!netdev_is_flow_api_enabled()) {
> +VLOG_DBG("Flow API is not enabled.");
> +return;
> +}
> +
> +if (psample_sock) {
> +VLOG_DBG("Psample socket is already initialized.");
> +return;
> +}
> +
> +err = nl_lookup_genl_family(PSAMPLE_GENL_NAME,
> +_family);
> +if (err) {
> +VLOG_WARN("Generic Netlink family '%s' does not exist: %s\n"
> + "Please make sure the kernel module psample is loaded.",
> + PSAMPLE_GENL_NAME, ovs_strerror(err));
> +return;
> +}
> +
> +err = nl_lookup_genl_mcgroup(PSAMPLE_GENL_NAME,
> + PSAMPLE_NL_MCGRP_SAMPLE_NAME,
> +
[ovs-dev] [PATCH v22 4/8] netdev-offload-tc: Add sFlow offload API for TC
Initialize psample socket. Add sFlow recv API to receive sampled
packets from psample socket. Add sFow recv wait API to add psample
socket fd to poll list.
Signed-off-by: Chris Mi
Reviewed-by: Roi Dayan
---
lib/dpif.h| 7 ++
lib/netdev-offload-provider.h | 11 ++
lib/netdev-offload-tc.c | 188 ++
3 files changed, 206 insertions(+)
diff --git a/lib/dpif.h b/lib/dpif.h
index 6cb4dae6d..95807af8f 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -836,6 +836,13 @@ struct dpif_upcall {
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. */
+/* SFlow offload only.
+ * When receiving sampled packets from psample socket, there is no
+ * flow key. But input tunnel and input ifindex are available. They
+ * are enough to construct flow and continue to process sFlow.
+ */
+struct flow_tnl *in_tun;/* Input tunnel key. */
+uint32_t iifindex; /* Input ifindex. */
};
/* A callback to notify higher layer of dpif about to be purged, so that
diff --git a/lib/netdev-offload-provider.h b/lib/netdev-offload-provider.h
index 9108856d1..9e2722fd1 100644
--- a/lib/netdev-offload-provider.h
+++ b/lib/netdev-offload-provider.h
@@ -121,6 +121,17 @@ struct netdev_flow_api {
int (*meter_del)(ofproto_meter_id meter_id,
struct ofputil_meter_stats *stats);
+/* Receives sampled packets in 'buf' from psample socket and fill the
+ * necessary members in 'upcall'.
+ * Return 0 if successful, otherwise returns a positive errno value.
+ */
+int (*sflow_recv)(struct dpif_upcall *upcall, struct ofpbuf *buf);
+
+/* Add psample socket fd to poll list. Wake the upcall thread up to
+ * process it if there is any sampled packets,
+ */
+void (*sflow_recv_wait)(void);
+
/* 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 0dbb7954f..d7901fa68 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -18,6 +18,8 @@
#include
#include
+#include
+#include
#include "dpif.h"
#include "hash.h"
@@ -104,6 +106,9 @@ static void parse_tc_flower_to_stats(struct tc_flower
*flower,
static int get_ufid_adjust_stats(const ovs_u128 *ufid,
struct dpif_flow_stats *stats);
+static struct nl_sock *psample_sock;
+static int psample_family;
+
/* When offloading sample action to TC, userspace creates a unique ID
* to map sFlow action and tunnel info and passes this ID to kernel
* instead of the sFlow info. Psample will send this ID and sampled
@@ -151,6 +156,19 @@ sgid_find(uint32_t id)
return node ? CONTAINER_OF(node, const struct sgid_node, id_node) : NULL;
}
+static struct offload_sflow *
+sflow_find(uint32_t id)
+{
+const struct sgid_node *node;
+
+node = sgid_find(id);
+if (!node) {
+return NULL;
+}
+
+return CONST_CAST(struct offload_sflow *, >sflow);
+}
+
static uint32_t
dpif_sflow_hash(const struct offload_sflow *sflow)
{
@@ -3015,6 +3033,55 @@ tc_cleanup_policer_actions(struct id_pool *police_ids,
hmap_destroy();
}
+static void
+psample_init(void)
+{
+unsigned int psample_mcgroup;
+int err;
+
+if (!netdev_is_flow_api_enabled()) {
+VLOG_DBG("Flow API is not enabled.");
+return;
+}
+
+if (psample_sock) {
+VLOG_DBG("Psample socket is already initialized.");
+return;
+}
+
+err = nl_lookup_genl_family(PSAMPLE_GENL_NAME,
+_family);
+if (err) {
+VLOG_WARN("Generic Netlink family '%s' does not exist: %s\n"
+ "Please make sure the kernel module psample is loaded.",
+ PSAMPLE_GENL_NAME, ovs_strerror(err));
+return;
+}
+
+err = nl_lookup_genl_mcgroup(PSAMPLE_GENL_NAME,
+ PSAMPLE_NL_MCGRP_SAMPLE_NAME,
+ _mcgroup);
+if (err) {
+VLOG_WARN("Failed to join Netlink multicast group '%s': %s",
+ PSAMPLE_NL_MCGRP_SAMPLE_NAME, ovs_strerror(err));
+return;
+}
+
+err = nl_sock_create(NETLINK_GENERIC, _sock);
+if (err) {
+VLOG_WARN("Failed to create psample socket: %s", ovs_strerror(err));
+return;
+}
+
+err = nl_sock_join_mcgroup(psample_sock, psample_mcgroup);
+if (err) {
+VLOG_WARN("Failed to join psample mcgroup: %s", ovs_strerror(err));
+nl_sock_destroy(psample_sock);
+psample_sock = NULL;
+return;
+}
+}
+
static int
netdev_tc_init_flow_api(struct netdev *netdev)
{
@@ -3069,6 +3136,7 @@ netdev_tc_init_flow_api(struct netdev *netdev)
