On 8 Jul 2021, at 9:41, Amber, Kumar wrote:
> Hi Eelco, > > > >> -----Original Message----- >> From: Eelco Chaudron <[email protected]> >> Sent: Thursday, July 8, 2021 1:00 PM >> To: Amber, Kumar <[email protected]> >> Cc: Ferriter, Cian <[email protected]>; [email protected]; >> [email protected]; [email protected]; Van Haaren, Harry >> <[email protected]>; Stokes, Ian <[email protected]> >> Subject: Re: [v6 06/11] dpif-netdev: Add packet count and core id paramters >> for >> study >> >> >> >> On 7 Jul 2021, at 17:16, Amber, Kumar wrote: >> >>> Hi Eelco, >>> >>> Don’t know the formatting keeps breaking . replies are inline. >>> >> >> <Snip> >>> + /* argv[2] is optional packet count, which user can provide along >>> + with >>> + * study function to set the minimum packet that must be matched in >>> + order >>> + * to choose the optimal function. */ uint32_t pkt_cmp_count = 0; >>> + uint32_t study_ret = 0; >>> + >>> + if ((argc == 3) || (argc == 4)) { >>> + if (str_to_uint(argv[2], 10, &pkt_cmp_count)) { study_ret = >>> + mfex_set_study_pkt_cnt(pkt_cmp_count, mfex_name); } else { study_ret >>> + = -EINVAL; >>> >>> An invalid input was given so we should error out. >>> >>> The error is handled later and since we already have a fallback to default >>> value >> we just fall-back. >> >> Don't think we should fallback to default, because if someone types: >> >> $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator >> THIS_PACKET_COUNT_PLEASE >> Miniflow implementation set to autovalidator >> >> We accept it and use the default value, it’s better to error out… >> > > For any other function other than study we simply set the request and ignore > the study_cnt in that case : > > root@npg-ngf-wlpr-srv06:~/amber# $OVS_DIR/utilities/ovs-appctl > dpif-netdev/miniflow-parser-set scalar 2056 > 2021-07-08T07:26:36Z|00101|dpif_netdev|INFO|Miniflow implementation set to > scalar. > Miniflow implementation set to scalar. > > For Study only if someone doesn’t prove a valid number we set the study-cnt > to default value . > > root@npg-ngf-wlpr-srv06:~/amber# $OVS_DIR/utilities/ovs-appctl > dpif-netdev/miniflow-parser-set -pmd 3 study abc > 2021-07-08T07:29:04Z|00103|dpif_netdev|INFO|Miniflow implementation set to > study, on pmd thread 3. > 2021-07-08T07:29:04Z|00007|dpif_mfex_extract_study(pmd-c03/id:7)|INFO|MFEX > study chose impl avx512_dot1q_ipv4_udp: (hits 128/128 pkts) > Miniflow implementation set to study, on pmd thread 3. I think we should inform the user of any mistake he makes, and not assume a default value. So in case, the count is supplied for a function not supporting it we should error. For example: $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator 13 The study_pkt_cnt option is not valid for the autovalidator implementation. ovs-appctl: ovs-vswitchd: server returned an error And for an invalid value to the study option: $ ovs-appctl dpif-netdev/miniflow-parser-set study WEWERWER Invalid value, “WEWERWER”, supplied for the study_pkt_cnt option. ovs-appctl: ovs-vswitchd: server returned an error Or something along these lines. >> >> <snip> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
