Re: [dpdk-dev] [PATCH v2] net/af_packet: fix vlan_insert corruption

[Ferruh Yigit <ferruh.yigit@intel.com>]

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/

> 
> Reported-by: Chas Williams <3chas3@gmail.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v2 - fix unnecessary parenthesis (was in original code)
> 
>  drivers/net/af_packet/rte_eth_af_packet.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
> index 99e13fe48a30..91ceb94ecbaa 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -200,6 +200,11 @@ eth_af_packet_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  			continue;
>  		}
>  
> +		/* point at the next incoming frame */
> +		if (ppd->tp_status != TP_STATUS_AVAILABLE &&
> +		    poll(&pfd, 1, -1) < 0)
> +			break;
> +
>  		/* insert vlan info if necessary */
>  		if (mbuf->ol_flags & PKT_TX_VLAN_PKT) {
>  			if (rte_vlan_insert(&mbuf)) {
> @@ -208,11 +213,6 @@ eth_af_packet_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
>  			}
>  		}
>  
> -		/* point at the next incoming frame */
> -		if ((ppd->tp_status != TP_STATUS_AVAILABLE) &&
> -		    (poll(&pfd, 1, -1) < 0))
> -			break;
> -
>  		/* copy the tx frame data */
>  		pbuf = (uint8_t *) ppd + TPACKET2_HDRLEN -
>  			sizeof(struct sockaddr_ll);
>