On 4/12/2019 11:08 PM, Stephen Hemminger wrote: > On Fri, 12 Apr 2019 17:28:17 +0100 > Ferruh Yigit <ferruh.yi...@intel.com> wrote: > >> On 4/8/2019 5:41 PM, Stephen Hemminger wrote: >>> If the af_packet transmit is sending a VLAN packet, >>> and the transmit path to the kernel os full, then it would >>> mismanage the outgoing mbuf. The original mbuf would end up >>> being freed twice, once by AF_PACKET PMD and once by caller. >> >> This comment is valid with your new patch [1] that updates >> 'rte_vlan_insert()' >> to duplicate the mbuf, right? >> >> That patch looks like won't make the release, so I suggest this one wait that >> patch, although this is harmless on its own, commit log is misleading. >> >> [1] >> https://patches.dpdk.org/patch/51870/ > > It was always true, even with existing vlan_insert. > Existing vlan_insert has issues if it ever creates a clone packet. > Existing vlan_insert can duplicate mbuf through clone >
Right, existing vlan_insert has same issue on af_packet. But, should vlan_insert try to duplicate the mbuf when it is shared, does it worth the complexity it brings? And when that support removed this patch won't be needed. Or perhaps can create a new API, that handles the shared mbuf and name explicitly states it? btw, 'continue' in our Tx loop is also not good, when the application gets less 'num_tx' because of 'continue' in 'vlan_insert' failure, it will think last packets in the array not sent and will try to free them which will cause double free again.
