On 3/1/2021 8:48 PM, Ilya Maximets wrote:
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.
Thanks for your above suggestion, Ilya. I'll check what need to be improved.
Regards,
Chris
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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
