On 05/03/2025 20:44, Ilya Maximets wrote:
> On 2/28/25 14:42, Kevin Traynor wrote:
>> Rename parse_vlan_push_action() to add_vlan_push_action()
>> as it is inconsistent with other add/parse action functions.
>>
>> Seen as it unconditionally returns 0 might as well change
>> to return void too.
>>
>> Also, a redundant return code check is removed.
>>
>> Signed-off-by: Kevin Traynor <ktray...@redhat.com>
>> ---
>> lib/netdev-offload-dpdk.c | 13 +++++--------
>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>> index d0b0d4c62..6ca271489 100644
>> --- a/lib/netdev-offload-dpdk.c
>> +++ b/lib/netdev-offload-dpdk.c
>> @@ -2076,7 +2076,7 @@ err:
>> }
>>
>> -static int
>> -parse_vlan_push_action(struct flow_actions *actions,
>> - const struct ovs_action_push_vlan *vlan_push)
>> +static void
>> +add_vlan_push_action(struct flow_actions *actions,
>> + const struct ovs_action_push_vlan *vlan_push)
>> {
>> struct rte_flow_action_of_push_vlan *rte_push_vlan;
>> @@ -2097,5 +2097,4 @@ parse_vlan_push_action(struct flow_actions *actions,
>> add_flow_action(actions, RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID,
>> rte_vlan_vid);
>> - return 0;
>> }
>>
>> @@ -2140,5 +2139,5 @@ parse_clone_actions(struct netdev *netdev,
>> } else if (clone_type == OVS_ACTION_ATTR_PUSH_VLAN) {
>> const struct ovs_action_push_vlan *vlan = nl_attr_get(ca);
>> - parse_vlan_push_action(actions, vlan);
>> + add_vlan_push_action(actions, vlan);
>> } else {
>> VLOG_DBG_RL(&rl,
>> @@ -2234,7 +2233,5 @@ parse_flow_actions(struct netdev *netdev,
>> const struct ovs_action_push_vlan *vlan = nl_attr_get(nla);
>>
>> - if (parse_vlan_push_action(actions, vlan)) {
>> - return -1;
>> - }
>> + add_vlan_push_action(actions, vlan);
>> } else if (nl_attr_type(nla) == OVS_ACTION_ATTR_POP_VLAN) {
>> add_flow_action(actions, RTE_FLOW_ACTION_TYPE_OF_POP_VLAN,
>> NULL);
>
> Acked-by: Ilya Maximets <i.maxim...@ovn.org>
>
Thanks David and Ilya. Applied and pushed.
> FWIW, if you want to clean this code even further, there is no real reason to
> have the parse_clone_actions() function. We should be just calling the main
> parse_flow_actions() recursively, AFAICT.
>
Thanks for the suggestion, I'll check more and send a patch to remove it
if i don't find a reason to keep.
thanks,
Kevin.
> Best regards, Ilya Maximets.
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
