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<[email protected]>
Reviewed-by: Roi Dayan<[email protected]>
---
lib/dpif.h | 7 ++
lib/netdev-offload-provider.h | 11 ++
lib/netdev-offload-tc.c | 188 ++++++++++++++++++++++++++++++++++
3 files changed, 206 insertions(+)
<snip>
+{
+ 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(&psample, buf);
+ if (!error) {
+ return psample_parse_packet(&psample, &sflow, 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(&psample, buf);
if (!error) {
return psample_parse_packet(&psample, 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(&psample, buf);
if (error) {
return error;
}
return psample_parse_packet(&psample, upcall);
Or even just a ternary operator:
error = psample_from_ofpbuf(&psample, buf);
return error ? error : psample_parse_packet(&psample, 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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
