Re: [PATCH 2/2] udp_flow: move all udp_flow functions to udp_flow.c

[Laurent Vivier <lvivier@redhat.com>]

On 03/08/2024 09:51, David Gibson wrote:
> On Fri, Aug 02, 2024 at 06:10:36PM +0200, Laurent Vivier wrote:
>> No code change.
>>
>> They need to be exported to be available by the vhost-user version of
>> passt.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> LGTM, with one nit:
> 
>> +/**
>> + * udp_at_sidx() - Get UDP specific flow at given sidx
>> + * @sidx:    Flow and side to retrieve
>> + *
>> + * Return: UDP specific flow at @sidx, or NULL of @sidx is invalid.  Asserts if
>> + *         the flow at @sidx is not FLOW_UDP.
>> + */
>> +struct udp_flow *udp_at_sidx(flow_sidx_t sidx)
>> +{
>> +	union flow *flow = flow_at_sidx(sidx);
>> +
>> +	if (!flow)
>> +		return NULL;
>> +
>> +	ASSERT(flow->f.type == FLOW_UDP);
>> +	return &flow->udp;
>> +}
> 
> udp_at_sidx() is so simple it probably makes more sense to have it as
> an inline in the header file, rather than a regular function.  When it
> was a local function, I was pretty much assuming the compiler would
> inline it anyways.
> 

I can't move it to udp_flow.h because udp_at_sidx() uses flow_at_sidx() that is defined in 
flow_table.h after udp_flow.h has already been included. flow_table.h includes udp_flow.h 
at its top because it needs "struct udp_flow" in union flow.

Thanks,
Laurent