[ovs-dev] 答复: [PATCH] netdev-dpdk: fix incorrect shinfo initialization

Yi Yang (杨燚)-云服务集团 yangyi01 at inspur.com
Wed Oct 28 00:35:02 UTC 2020


-----邮件原件-----
发件人: dev [mailto:ovs-dev-bounces at openvswitch.org] 代表 Flavio Leitner
发送时间: 2020年10月27日 21:08
收件人: Ilya Maximets <i.maximets at ovn.org>
抄送: yang_y_yi at 163.com; ovs-dev at openvswitch.org; olivier.matz at 6wind.com
主题: Re: [ovs-dev] [PATCH] netdev-dpdk: fix incorrect shinfo initialization

On Tue, Oct 27, 2020 at 01:47:22PM +0100, Ilya Maximets wrote:
> On 10/27/20 12:34 PM, Flavio Leitner wrote:
> > On Wed, Oct 14, 2020 at 03:22:48PM +0800, yang_y_yi at 163.com wrote:
> >> From: Yi Yang <yangyi01 at inspur.com>
> >>
> >> shinfo is used to store reference counter and free callback of an 
> >> external buffer, but it is stored in mbuf if the mbuf has tailroom 
> >> for it.
> >>
> >> This is wrong because the mbuf (and its data) can be freed before 
> >> the external buffer, for example:
> >>
> >>   pkt2 = rte_pktmbuf_alloc(mp);
> >>   rte_pktmbuf_attach(pkt2, pkt);
> >>   rte_pktmbuf_free(pkt);
> 
> How is that possible with OVS?  Right now OVS doesn't support 
> multi-segement mbufs and will, likely, not support them in a near 
> future because it requires changes all other the codebase.
> 
> Is there any other scenario that could lead to issues with current OVS 
> implementation?

This is copying packets. The shinfo is allocated in the mbuf of the first packet which could be deleted without any references to the external buffer still using it.

Fbl

[Yi Yang]  Yes, this is not related with multi-segment mbuf, dpdk interfaces to system interfaces communication will use it if the packet size is greater than mtu size, i.e. TSO case from veth/tap to dpdk/vhost and backward will use it, this is a wrong use of shinfo, the same fix (which is used by virtio/vhost driver)has been merged into dpdk branch.
 
[Yi Yang] 



> 
> >>
> >> After this, pkt is freed, but it still contains shinfo, which is 
> >> referenced by pkt2.
> >>
> >> Fix this by always storing shinfo at the tail of external buffer.
> >>
> >> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload 
> >> support")
> >> Co-authored-by: Olivier Matz <olivier.matz at 6wind.com>
> >> Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
> >> Signed-off-by: Yi Yang <yangyi01 at inspur.com>
> >> ---
> > 
> > Acked-by: Flavio Leitner <fbl at sysclose.org>
> > 
> > Thanks Yi,
> > fbl
> > 
> 

--
fbl
_______________________________________________
dev mailing list
dev at openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list