> +static inline bool
> +tap_mac_filter_match(struct rx_queue *rxq, struct rte_mbuf *mbuf)
> +{
> +     struct pmd_internals *pmd = rxq->pmd;
> +     struct rte_eth_dev_data *data;
> +     struct rte_ether_addr *dst;
> +     uint32_t i;
> +
> +     if (!pmd->macfilter)
> +             return true;
> +
> +     data = pmd->dev->data;
> +     if (data->promiscuous)
> +             return true;
> +
> +     dst = rte_pktmbuf_mtod(mbuf, struct rte_ether_addr *);
> +
> +     if (rte_is_broadcast_ether_addr(dst))
> +             return true;
> +
> +     if (rte_is_multicast_ether_addr(dst)) {
> +             if (data->all_multicast)
> +                     return true;
> +             for (i = 0; i < pmd->nb_mc_addrs; i++) {
> +                     if (rte_is_same_ether_addr(dst, &pmd->mc_addrs[i]))
> +                             return true;
> +             }
> +             return false;
> +     }
> +
> +     for (i = 0; i < TAP_MAX_MAC_ADDRS; i++) {
> +             if (rte_is_zero_ether_addr(&data->mac_addrs[i]))
> +                     continue;
> +             if (rte_is_same_ether_addr(dst, &data->mac_addrs[i]))
> +                     return true;
> +     }
> +     return false;
> +}

Performance optimization; the vast majority of packets are unicast:

if (likely(rte_is_unicast_ether_addr(dst)) {
    if (unlikely(rte_is_zero_ether_addr(dst)))
        return false; /* Invalid destination address in packet */
    /* Note: Compiler loop unrolls. */
    for (i = 0; i < TAP_MAX_MAC_ADDRS; i++) {
        if (rte_is_same_ether_addr(dst, &data->mac_addrs[i]))
            return true;
    }
}

if (data->all_multicast)
    return true;

for (i = 0; i < pmd->nb_mc_addrs; i++) {
    if (rte_is_same_ether_addr(dst, &pmd->mc_addrs[i]))
        return true;
}

if (rte_is_broadcast_ether_addr(dst))
    return true;

return false;

Also, all the variables and function parameters could be "const"; although I 
don't know if it makes any practical difference.


>  static int
> -tap_set_mc_addr_list(struct rte_eth_dev *dev __rte_unused,
> -                  struct rte_ether_addr *mc_addr_set __rte_unused,
> -                  uint32_t nb_mc_addr __rte_unused)
> +tap_set_mc_addr_list(struct rte_eth_dev *dev,
> +                  struct rte_ether_addr *mc_addr_set,
> +                  uint32_t nb_mc_addr)
>  {
> -     /*
> -      * Nothing to do actually: the tap has no filtering whatsoever,
> every
> -      * packet is received.
> -      */
> +     struct pmd_internals *pmd = dev->data->dev_private;
> +
> +     if (nb_mc_addr == 0) {
> +             rte_free(pmd->mc_addrs);
> +             pmd->mc_addrs = NULL;
> +             pmd->nb_mc_addrs = 0;
> +             return 0;
> +     }
> +
> +     pmd->mc_addrs = rte_realloc(pmd->mc_addrs,
> +                                 nb_mc_addr * sizeof(*pmd->mc_addrs), 0);
> +     if (pmd->mc_addrs == NULL) {
> +             pmd->nb_mc_addrs = 0;
> +             return -ENOMEM;
> +     }
> +
> +     memcpy(pmd->mc_addrs, mc_addr_set,
> +            nb_mc_addr * sizeof(*pmd->mc_addrs));
> +     pmd->nb_mc_addrs = nb_mc_addr;
> +
>       return 0;
>  }

This is not thread safe.
If it needs to be thread safe, you must ensure that tap_mac_filter_match() - 
running on another lcore - is done using the mc_addrs array before you free it 
here.

Reply via email to