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.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to