On 6/23/21 5:48 PM, Eli Britstein wrote:
>>>> @@ -1009,7 +1009,7 @@ parse_vxlan_match(struct flow_patterns *patterns,
>>>> return 0;
>>>> }
>>>>
>>>> -static int
>>>> +static int OVS_UNUSED
>
> Note that if experimental is allowed, the OVS_UNUSED attribute is misleading.
Yeah, I know. We might introduce OVS_MAY_BE_UNUSED macro someday, but that is
really minor.
>
> Also see below.
>
>>>> parse_flow_tnl_match(struct netdev *tnldev,
>>>> struct flow_patterns *patterns,
>>>> odp_port_t orig_in_port,
>>>> @@ -1031,7 +1031,7 @@ parse_flow_tnl_match(struct netdev *tnldev,
>>>>
>>>> static int
>>>> parse_flow_match(struct netdev *netdev,
>>>> - odp_port_t orig_in_port,
>>>> + odp_port_t orig_in_port OVS_UNUSED,
>>>> struct flow_patterns *patterns,
>>>> struct match *match)
>>>> {
>>>> @@ -1045,10 +1045,12 @@ parse_flow_match(struct netdev *netdev,
>>>> }
>>>>
>>>> patterns->physdev = netdev;
>>>> +#ifdef ALLOW_EXPERIMENTAL_API /* Packet restoration API required. */
>
> In my opinion those should be removed in netdev-offload-dpdk.c, and keep such
> #ifdef only in netdev-dpdk (with stubs), so later, when dpdk removes the
> experimental attribute, there will be a single place to change.
>
> This applies both to parse_flow_tnl_match and add_tnl_pop_action.
>
> However, this is not critical and I would not hold the merge because of this.
I agree that it's a bit of an overthinking from my side, but we will
need to introduce this kind of guarding here if DPDK APIs will become
non-experimental not all at once. I'm not sure if that is a possible
scenario, but just in case. Looking more at the code, I agree that
they are unnecessary for current version, but let them be, as they will
remind us to re-check things once some of APIs will become stable.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
