[ovs-dev] [PATCH v12 00/11] Add offload support for sFlow
Ilya Maximets
i.maximets at ovn.org
Mon Mar 1 12:48:03 UTC 2021
On 3/1/21 9:30 AM, Chris Mi wrote:
> Hi Simon, Ilya,
>
> Could I know what should we do to make progress for this patch set?
> It has been posted in the community for a long time 😁
In general, the way to get your patches reviewed is to review other
patches. It's simply because we still have a huge review backlog
(214 patches right now in patchwork and most of them needs review)
and bugfixes usually has a bit higher priority. By reviewing other
patches you're reducing amount of work for maintainers so they can
get to your patches faster.
For the series and comments from Eelco:
I didn't read the patches carefully, only a quick glance, but I still
do not understand why we need a separate thread to poll psample events.
Why can't we just allow usual handler threads to do that? From the
architecture perspective it's not a good thing to call ofproto code
from the offload-provider. This introduces lots of complications
and might cause lots of issues in the future.
I'd say that design should look like this:
handler thread ->
dpif_recv() ->
dpif_netlink_recv() ->
netdev_offload_recv() ->
netdev_offload_tc_recv() ->
nl_sock_recv()
Last three calls should be implemented.
The better version of this will be to throw away dpif part from
above call chain and make it:
handler thread ->
netdev_offload_recv() ->
netdev_offload_tc_recv() ->
nl_sock_recv()
This way we could avoid touching dpif-netdev and still have psample
offloading for the case where netdev-offload-tc is used from the
userspace datapath.
Above architecture also implies implementation of:
- netdev_offload_recv_wait()
- netdev_offload_recv_purge()
- and the netdev_offload_tc_* counterparts.
Best regards, Ilya Maximets.
>
> Thanks,
> Chris
>
> On 2/23/2021 5:08 PM, Roi Dayan wrote:
>>
>>
>> On 2021-01-27 8:23 AM, Chris Mi wrote:
>>> This patch set adds offload support for sFlow.
>>>
>>> Psample is a genetlink channel for packet sampling. TC action act_sample
>>> uses psample to send sampled packets to userspace.
>>>
>>> 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 packet to
>>> userspace. Using the ID, userspace can recover the sFlow info and send
>>> sampled packet to the right sFlow monitoring host.
>>>
>>> v2-v1:
>>> - Fix robot errors.
>>> v3-v2:
>>> - Remove Gerrit Change-Id.
>>> - Add patch #9 to fix older kernels build issue.
>>> - Add travis test result.
>>> v4-v3:
>>> - Fix offload issue when sampling rate is 1.
>>> v5-v4:
>>> - Move polling thread from ofproto to netdev-offload-tc.
>>> v6-v5:
>>> - Rebase.
>>> - Add GitHub Actions test result.
>>> v7-v6:
>>> - Remove Gerrit Change-Id.
>>> - Fix "ERROR: Inappropriate spacing around cast"
>>> v8-v7
>>> - Address Eelco Chaudron's comment for patch #11.
>>> v9-v8
>>> - Remove sflow_len from struct dpif_sflow_attr.
>>> - Log a debug message for other userspace actions.
>>> v10-v9
>>> - Address Eelco Chaudron's comments on v9.
>>> v11-v10
>>> - Fix a bracing error.
>>> v12-v11
>>> - Add duplicate sample group id check.
>>>
>>> Chris Mi (11):
>>> compat: Add psample and tc sample action defines for older kernels
>>> ovs-kmod-ctl: Load kernel module psample
>>> dpif: Introduce register sFlow upcall callback API
>>> ofproto: Add upcall callback to process sFlow packet
>>> netdev-offload: Introduce register sFlow upcall callback API
>>> netdev-offload-tc: Implement register sFlow upcall callback API
>>> dpif-netlink: Implement register sFlow upcall callback API
>>> netdev-offload-tc: Introduce group ID management API
>>> netdev-offload-tc: Create psample netlink socket
>>> netdev-offload-tc: Add psample receive handler
>>> netdev-offload-tc: Add offload support for sFlow
>>>
>>> include/linux/automake.mk | 4 +-
>>> include/linux/psample.h | 58 +++
>>> include/linux/tc_act/tc_sample.h | 25 ++
>>> lib/dpif-netdev.c | 1 +
>>> lib/dpif-netlink.c | 27 ++
>>> lib/dpif-netlink.h | 4 +
>>> lib/dpif-provider.h | 10 +
>>> lib/dpif.c | 8 +
>>> lib/dpif.h | 23 ++
>>> lib/netdev-offload-provider.h | 3 +
>>> lib/netdev-offload-tc.c | 659 ++++++++++++++++++++++++++++++-
>>> lib/netdev-offload.c | 30 ++
>>> lib/netdev-offload.h | 4 +
>>> lib/tc.c | 61 ++-
>>> lib/tc.h | 16 +-
>>> ofproto/ofproto-dpif-upcall.c | 42 ++
>>> utilities/ovs-kmod-ctl.in | 14 +
>>> 17 files changed, 973 insertions(+), 16 deletions(-)
>>> create mode 100644 include/linux/psample.h
>>> create mode 100644 include/linux/tc_act/tc_sample.h
>>>
>>
>>
>> Hi Simon, Ilya,
>>
>> Can you help review for this series?
>> do you have any comments you want us to handle?
>>
>> Thanks,
>> Roi
>
More information about the dev
mailing list