On 6/14/24 13:33, Simon Jones wrote:
> ```
> Date: Fri Jun 14 19:25:43 2024 +0800
>
> bugfix of meter tc crash.
>
> diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
> index 9fde5f7a9..d08c5a35f 100644
> --- a/lib/netdev-offload.c
> +++ b/lib/netdev-offload.c
> @@ -186,7 +186,6 @@ netdev_assign_flow_api(struct netdev *netdev)
> ovsrcu_set(&netdev->flow_api, rfa->flow_api);
> VLOG_INFO("%s: Assigned flow API '%s'.",
> netdev_get_name(netdev), rfa->flow_api->type);
> - return 0;
> }
> VLOG_DBG("%s: flow API '%s' is not suitable.",
> netdev_get_name(netdev), rfa->flow_api->type);
> ```
>
> ----
> Simon Jones
>
>
> Simon Jones <batmanu...@gmail.com> 于2024年6月14日周五 19:25写道：
>
>> Maybe reason is this:
>> ```
>> @netdev_offload_tc and @netdev_offload_dpdk will always be register.
>> Then the @meter_set api will be called.
>> The @meter_set could be called only after @init_flow_api, but the BUG
>> happens when @meter_set is called before @init_flow_api is called.
>>
>> Check these code:
>>
>> static int
>> netdev_assign_flow_api(struct netdev *netdev)
>> {
>> struct netdev_registered_flow_api *rfa;
>>
>> CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) {
>> if (!rfa->flow_api->init_flow_api(netdev)) {
>> ovs_refcount_ref(&rfa->refcnt);
>> ovsrcu_set(&netdev->flow_api, rfa->flow_api);
>> VLOG_INFO("%s: Assigned flow API '%s'.",
>> netdev_get_name(netdev), rfa->flow_api->type);
>> return 0;
>> }
>> VLOG_DBG("%s: flow API '%s' is not suitable.",
>> netdev_get_name(netdev), rfa->flow_api->type);
>> }
>> VLOG_INFO("%s: No suitable flow API found.", netdev_get_name(netdev));
>>
>> return -1;
>> }
>>
>> ONLY one type of rfa->flow_api->init_flow_api (DPDK or TC) could be called.
>> Because after one called, then return 0.
>> ```
>>
>> So I suggest to just remove 'return 0'.
Hi, Simon. This is not a correct thing to do. By design, only one flow API
should be able to accept a particular port type. If two API implementations
can accept the same port, that would be a bug. Also, initializing more than
one API will cause resource leaks and potentially incorrect datapath behavior.
Since you're using userspace datapath, init_flow_api() from the TC
implementation
should always fail.
What is your OVS version?
Best regards, Ilya Maximets.
>> Like this patch:
>>
>> ----
>> Simon Jones
>>
>>
>> Simon Jones <batmanu...@gmail.com> 于2024年6月11日周二 17:32写道：
>>
>>>
>>> Hi all,
>>>
>>> I'm using ovs-dpdk with this patch:
>>> ```
>>> commit 4c226944f7c55c9d6e7c85f7c33c5ce11c35ce54
>>> Author: Jianbo Liu <jian...@nvidia.com>
>>> Date: Fri Jul 8 03:06:26 2022 +0000
>>>
>>> netdev-offload-tc: Implement meter offload API for tc
>>> ```
>>> This patch is for offload flow meter by tc.
>>>
>>> Now I found a bug: ovs crash when add meter openflow.
>>>
>>> 1. How to produce:
>>> (NOTICE: This bug is not always reproducible.)
>>> ```
>>> Add these commands:
>>>
>>> ovs-ofctl -O OpenFlow13 add-meter br-int
>>> meter=1,kbps,band=type=drop,rate=1000
>>> ovs-ofctl -O OpenFlow13 add-flow br-int in_port=\"pf0vf0\",ip,nw_src=
>>> 16.0.0.0/24,nw_dst=48.0.0.0/24,nw_proto=17,actions=\
>>> <http://16.0.0.0/24,nw_dst=48.0.0.0/24,nw_proto=17,actions=%5C>
>>> "meter:1,output:p0\"
>>> ovs-ofctl -O OpenFlow13 add-flow br-int
>>> in_port=\"pf0vf0\",udp6,ipv6_src=2001:4860:0:2001::/64,ipv6_dst=2001:0:4137:9350:8000:f12a:b9c8:2815,actions=\"meter:1,output:p0\"
>>>
>>> Then ovs crash, this is core file call trace:
>>>
>>> (gdb) bt
>>> #0 id_pool_alloc_id (pool=0x0, id_=id_@entry=0x7fff180c841c) at
>>> lib/id-pool.c:112
>>> #1 0x000000000055f0a0 in meter_alloc_police_index
>>> (police_index=0x7fff180c841c) at lib/netdev-offload-tc.c:2530
>>> #2 meter_tc_set_policer (meter_id=..., config=0x7fff180c8538) at
>>> lib/netdev-offload-tc.c:2567
>>> #3 0x00000000004af207 in meter_offload_set (meter_id=meter_id@entry=...,
>>> config=config@entry=0x7fff180c8538) at lib/netdev-offload.c:207
>>> #4 0x0000000000474c2b in dpif_netdev_meter_set (dpif=<optimized out>,
>>> meter_id=..., config=0x7fff180c8538) at lib/dpif-netdev.c:9137
>>> #5 0x000000000048e3fd in dpif_meter_set (dpif=0x28cfd10, meter_id=...,
>>> config=<optimized out>) at lib/dpif.c:1973
>>> #6 0x000000000042daf8 in meter_set (ofproto_=0x28cc670,
>>> meter_id=0x7fff180c8590, config=<optimized out>) at
>>> ofproto/ofproto-dpif.c:6751
>>> #7 0x0000000000423e91 in handle_add_meter (mm=0x7fff180c8530,
>>> ofproto=0x28cc670) at ofproto/ofproto.c:6905
>>> #8 handle_meter_mod (ofconn=0x29073d0, oh=0x291bd40) at
>>> ofproto/ofproto.c:7013
>>> #9 0x0000000000426d21 in handle_single_part_openflow
>>> (type=OFPTYPE_METER_MOD, oh=0x291bd40, ofconn=0x29073d0) at
>>> ofproto/ofproto.c:8668
>>> #10 handle_openflow (ofconn=0x29073d0, msgs=0x7fff180c88c0) at
>>> ofproto/ofproto.c:8843
>>> #11 0x000000000045b3b4 in ofconn_run (handle_openflow=0x4267b0
>>> <handle_openflow>, ofconn=0x29073d0) at ofproto/connmgr.c:1329
>>> #12 connmgr_run (mgr=0x272a200,
>>> handle_openflow=handle_openflow@entry=0x4267b0
>>> <handle_openflow>) at ofproto/connmgr.c:356
>>> #13 0x00000000004209be in ofproto_run (p=0x28cc670) at
>>> ofproto/ofproto.c:1964
>>> #14 0x000000000040da44 in bridge_run__ () at vswitchd/bridge.c:3251
>>> #15 0x00000000004138ec in bridge_run () at vswitchd/bridge.c:3310
>>> #16 0x000000000040955d in main (argc=<optimized out>, argv=<optimized
>>> out>) at vswitchd/ovs-vswitchd.c:129
>>>
>>> ```
>>>
>>> 2. Check code:
>>> ```
>>> Notice
>>> #0 id_pool_alloc_id (pool=0x0, id_=id_@entry=0x7fff180c841c) at
>>> lib/id-pool.c:112
>>> the @pool param is NULL.
>>>
>>> This is happened ONLY when @netdev_tc_init_flow_api is NOT called.
>>> But in code, ONLY after @netdev_tc_init_flow_api is called, the
>>> @handle_meter_mod could works on netdev with system type.
>>> ```
>>>
>>> 3. My question:
>>> - How this BUG happen?
>>> - Is there patch to solve this BUG?
>>>
>>>
>>> ----
>>> Simon Jones
>>>
>>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
