Hi Yi,
Thanks for putting this patch-set together.
On Wed, Jul 01, 2020 at 05:15:29PM +0800, yang_y...@163.com wrote:
> From: Yi Yang <yangy...@inspur.com>
>
> For multi-seg mbuf, pkt_len isn't equal to data_len,
> data_len is data_len of the first seg, pkt_len is
> sum of data_len of all the segs, so for such packets,
> dp_packet_set_size shouldn't change data_len.
>
> Signed-off-by: Yi Yang <yangy...@inspur.com>
> ---
> lib/dp-packet.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 0430cca..070d111 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -575,7 +575,9 @@ dp_packet_set_size(struct dp_packet *b, uint32_t v)
> * (and thus 'v') will always be <= UINT16_MAX; this means that there is
> no
> * loss of accuracy in assigning 'v' to 'data_len'.
> */
> - b->mbuf.data_len = (uint16_t)v; /* Current seg length. */
> + if (b->mbuf.nb_segs <= 1) {
> + b->mbuf.data_len = (uint16_t)v; /* Current seg length. */
> + }
> b->mbuf.pkt_len = v; /* Total length of all segments linked
> to
> * this segment. */
> }
Currently OVS doesn't support multi-seg mbuf, so although
this patch wouldn't break anything it doesn't sound correct
as it is. It seems incomplete/limited as well.
I think at least the patch should add a comment explaining
why and when that is needed.
Another thing is that this change alone has no users. Usually
we do changes along with the first user. I am still reviewing
the following patches, but I suspect this change is for GRO/GSO,
so in my opinion it makes sense to be part of one of them.
Doing so helps to backtrack the reason for a specific change.
I think we should prioritize single mbuf as they carry less
data, so OVS has less time to process them. Therefore, it
seems appropriate to use OVS_LIKELY().
--
fbl
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
