On 6/24/2023 4:18 AM, Ilya Maximets wrote:
On 6/19/23 07:05, 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.
v13-v12
- Remove the psample poll thread from netdev-offload-tc and reuse
   ofproto handler thread according to Ilya's new desgin.
- Add dpif-offload-provider layer according to Eli's suggestion.
v14-v13
- Fix a robot error.
v15-v14
- Address Eelco Chaudron's comments on v14.
v16-v15
- Address Eelco Chaudron's comments on v15.
- Add two test cases.
v17-v16
- Address Eelco Chaudron's comments on v16.
- Move struct dpif_offload_api from struct dpif_class to struct dpif.
v18-v17
- Rename dpif_offload_api to dpif_offload_class.
- Add init and destroy callbacks in dpif_offload_class. They are called
   when registering dpif_offload_class.
v19-18
- Fix a bug that psample_sock is destroyed when last bridge is deleted.
v20-19
- Move buf_stub to struct dpif_offload_sflow avoid garbage values when
   ofproto proceses the sampled packet.
v21-20
- Remove netdev dummy for dpif-offload according to Eelco's comment.
v22-21
- Address Ilya's comments:
  - Remove dpif-offload-provider layer.
  - Remove process_offload_sflow and reuse upcall_receive.
  - Introduce sample id pool.
  - Introduce netdev_offload_recv.
v23-22
- Address Ilya's comments:
  - Add struct flow in struct dpif_upcall.
  - Add handler id in recv() and recv_wait().
  - misc changes.
v24-23
- Fix checkpath and actions errors.
v25-24
- Address Eelco's comments:
  - Add tunnel test.
  - Change sample group id and sample info to 1:1 mapping.
  - Move sample group id from tcf_id to ufid_to_tc_data.
  - misc changes.
v26-25
- Address Eelco's comments:
  - Rename actions to userspace_actions.
  - Add in tunnel test.
v27-26
- Address Eelco's comments:
  - Introduce nullable_xmemdup() and flow_tnl_copy().
  - Improve sgid node locking.
  - Improve comments and documents.
  - Add support for multiple sgids.
  - Improve test cases.
  - misc changes.
v28-27
- Apply Eelco's three diff files directly.

Chris Mi (8):
   compat: Add psample and tc sample action defines for older kernels
   ovs-kmod-ctl: Load kernel module psample
   netdev-offload-tc: Introduce group ID management API
   netdev-offload-tc: Add sample offload API for TC
   netdev-offload: Add netdev offload recv and recv_wait APIs
   dpif-netlink: Add netdev offload recv in normal recv upcalls
   netdev-offload-tc: Add offload support for sFlow
   system-offloads-traffic.at: Add sFlow offload test cases

  Documentation/howto/tc-offload.rst |  24 ++
  include/linux/automake.mk          |  10 +-
  include/linux/psample.h            |  62 +++
  include/linux/tc_act/tc_sample.h   |  25 ++
  lib/dpif-netlink.c                 |  41 +-
  lib/dpif.h                         |   6 +-
  lib/flow.h                         |   2 +-
  lib/netdev-offload-provider.h      |  30 ++
  lib/netdev-offload-tc.c            | 598 +++++++++++++++++++++++++++--
  lib/netdev-offload.c               |  41 +-
  lib/netdev-offload.h               |   5 +
  lib/packets.h                      |   2 +-
  lib/tc.c                           |  63 ++-
  lib/tc.h                           |   6 +
  lib/util.c                         |   6 +
  lib/util.h                         |   1 +
  ofproto/ofproto-dpif-upcall.c      |  15 +-
  tests/system-common-macros.at      |  13 +
  tests/system-offloads-traffic.at   | 160 ++++++++
  utilities/ovs-kmod-ctl.in          |  14 +
  20 files changed, 1079 insertions(+), 45 deletions(-)
  create mode 100644 include/linux/psample.h
  create mode 100644 include/linux/tc_act/tc_sample.h



Hi, Chris, others.  I see a significant memory leak reported by ASAN
while running check_pkt_len offload tests:

13: offloads - check_pkt_len action - offloads enabled FAILED 
(ovs-macros.at:222)
=================================================================
==1258509==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1768 byte(s) in 26 object(s) allocated from:
     #0 0x555cedbe29be in malloc (/root/ovs/vswitchd/ovs-vswitchd+0x7949be) 
(BuildId: 5753749b2b8d499f7875b324b2c317ceb53b600f)
     #1 0x555cee37582c in xmalloc__ /root/ovs/lib/util.c:140:15
     #2 0x555cee37582c in xmalloc /root/ovs/lib/util.c:175:12
     #3 0x555cee445d48 in offload_sample_init 
/root/ovs/lib/netdev-offload-tc.c:2150:33
     #4 0x555cee445719 in parse_userspace_action 
/root/ovs/lib/netdev-offload-tc.c:2215:5
     #5 0x555cee43ce82 in netdev_tc_parse_nl_actions 
/root/ovs/lib/netdev-offload-tc.c:2507:23
     #6 0x555cee421e00 in netdev_tc_flow_put 
/root/ovs/lib/netdev-offload-tc.c:2818:11
     #7 0x555cee3b2fc6 in parse_flow_put /root/ovs/lib/dpif-netlink.c:2307:11
     #8 0x555cee3b2fc6 in try_send_to_netdev 
/root/ovs/lib/dpif-netlink.c:2394:15
     #9 0x555cee3b2fc6 in dpif_netlink_operate 
/root/ovs/lib/dpif-netlink.c:2465:23
     #10 0x555cedf36bc8 in dpif_operate /root/ovs/lib/dpif.c:1376:13
     #11 0x555cedd7c731 in push_dp_ops 
/root/ovs/ofproto/ofproto-dpif-upcall.c:2496:5
     #12 0x555cedd9221d in revalidate 
/root/ovs/ofproto/ofproto-dpif-upcall.c:2904:13
     #13 0x555cedd7ef54 in udpif_revalidator 
/root/ovs/ofproto/ofproto-dpif-upcall.c:1017:9
     #14 0x555cee23f972 in ovsthread_wrapper /root/ovs/lib/ovs-thread.c:423:12
     #15 0x7f59cb090401 in start_thread nptl/pthread_create.c:442:8

SUMMARY: AddressSanitizer: 1768 byte(s) leaked in 26 allocation(s).

Could you, please, take a look?  Impression is that we leak an entry
on each flow_put somehow.

Best regards, Ilya Maximets.

I can't reproduce it. Anyway, I found something wrong and 'fixed' it.
Thanks for your review, Ilya. I'll send v29 for review.

-Chris
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to