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