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);
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
