Flavio, thank you so much for reviewing, I'll fix them per your comments in next version. Some replies per your comments inline.
-----邮件原件----- 发件人: dev [mailto:ovs-dev-boun...@openvswitch.org] 代表 Flavio Leitner 发送时间: 2020年7月11日 3:42 收件人: yang_y...@163.com 抄送: ovs-dev@openvswitch.org 主题: Re: [ovs-dev] [PATCH v2 1/5] Fix dp_packet_set_size error for multi-seg mbuf 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(). [Yi Yang] Make sense, I'll merge it in GRO patch, yes, GRO needs it, OVS_LIKELY will be better. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
