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 <3ch...@gmail.com>
> Signed-off-by: Stephen Hemminger <step...@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);
> 

Reply via email to