| Message ID | 1384466519-24373-1-git-send-email-robherring2@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 11/14/2013 02:01 PM, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > The Calxeda xgmac must learn secondary mac addresses such as those > behind a bridge in order to properly receive packets with those mac > addresses. Add mac address learning on transmit with timestamping of > entries. The mac addresses are added to the driver's unicast address > list, so they are visible to user via "bridge fdb show" command. > > Addresses can get disabled when the AE bit is cleared, so address > registers must be checked for possible removal and update the unicast > list. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > --- > drivers/net/ethernet/calxeda/xgmac.c | 128 ++++++++++++++++++++++++++++++++++- > 1 file changed, 127 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c > index 48f5288..584aca8 100644 > --- a/drivers/net/ethernet/calxeda/xgmac.c > +++ b/drivers/net/ethernet/calxeda/xgmac.c > @@ -360,6 +360,13 @@ struct xgmac_extra_stats { > unsigned long rx_process_stopped; > unsigned long tx_early; > unsigned long fatal_bus_error; > + unsigned long learning_drop; > +}; > + > +struct xgmac_mac { > + char mac_addr[ETH_ALEN]; > + unsigned long time; > + bool rm_pending; > }; > > struct xgmac_priv { > @@ -384,6 +391,8 @@ struct xgmac_priv { > struct napi_struct napi; > > int max_macs; > + struct xgmac_mac mac_list[32]; > + > struct xgmac_extra_stats xstats; > > spinlock_t stats_lock; > @@ -392,6 +401,7 @@ struct xgmac_priv { > char tx_pause; > int wolopts; > struct work_struct tx_timeout_work; > + struct delayed_work mac_aging_work; > }; > > /* XGMAC Configuration Settings */ > @@ -634,7 +644,7 @@ static void xgmac_set_mac_addr(void __iomem *ioaddr, unsigned char *addr, > } > } > > -static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, > +static bool xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, > int num) > { > u32 hi_addr, lo_addr; > @@ -650,6 +660,8 @@ static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, > addr[3] = (lo_addr >> 24) & 0xff; > addr[4] = hi_addr & 0xff; > addr[5] = (hi_addr >> 8) & 0xff; > + > + return (hi_addr & XGMAC_ADDR_AE) ? true : false; > } > > static int xgmac_set_flow_ctrl(struct xgmac_priv *priv, int rx, int tx) > @@ -1047,6 +1059,8 @@ static int xgmac_open(struct net_device *dev) > writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS); > writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA); > > + schedule_delayed_work(&priv->mac_aging_work, 5 * HZ); > + > return 0; > } > > @@ -1059,6 +1073,7 @@ static int xgmac_open(struct net_device *dev) > static int xgmac_stop(struct net_device *dev) > { > struct xgmac_priv *priv = netdev_priv(dev); > + int i; > > netif_stop_queue(dev); > > @@ -1073,9 +1088,111 @@ static int xgmac_stop(struct net_device *dev) > /* Release and free the Rx/Tx resources */ > xgmac_free_dma_desc_rings(priv); > > + cancel_delayed_work_sync(&priv->mac_aging_work); > + for (i = 0; i < priv->max_macs; i++) { > + if (!is_valid_ether_addr(priv->mac_list[i].mac_addr)) > + continue; > + dev_uc_del(dev, priv->mac_list[i].mac_addr); > + memset(&priv->mac_list[i], 0, sizeof(priv->mac_list[i])); > + } > + > return 0; > } > > +static void xgmac_check_mac_addrs(struct xgmac_priv *priv) > +{ > + int i, j; > + char addr[ETH_ALEN]; > + > + for (i = 1; i < priv->max_macs; i++) { > + if (xgmac_get_mac_addr(priv->base, addr, i)) > + continue; > + > + if (!is_valid_ether_addr(addr)) > + break; > + > + for (j = 1; j < priv->max_macs; j++) { > + if (!ether_addr_equal(addr, priv->mac_list[j].mac_addr)) > + continue; > + priv->mac_list[j].rm_pending = true; > + break; > + } > + } > +} > + > +static bool xgmac_mac_rm_pending(struct xgmac_priv *priv, const u8 *addr) > +{ > + int i; > + > + for (i = 1; i < priv->max_macs; i++) { > + if (ether_addr_equal(addr, priv->mac_list[i].mac_addr) && > + priv->mac_list[i].rm_pending) > + return true; > + } > + return false; > +} > + > +static void xgmac_mac_aging_work(struct work_struct *work) > +{ > + int i; > + struct xgmac_priv *priv = > + container_of(work, struct xgmac_priv, mac_aging_work.work); > + struct net_device *dev = priv->dev; > + > + xgmac_check_mac_addrs(priv); > + > + for (i = 1; i < priv->max_macs; i++) { > + if (!priv->mac_list[i].rm_pending) > + continue; > + > + dev_uc_del(dev, priv->mac_list[i].mac_addr); > + priv->mac_list[i].rm_pending = false; > + priv->mac_list[i].time = 0; > + memset(priv->mac_list[i].mac_addr, 0, ETH_ALEN); > + } > + > + schedule_delayed_work(to_delayed_work(work), 10 * HZ); > +} > + > +static void xgmac_learn_mac(struct xgmac_priv *priv, struct sk_buff *skb) > +{ > + struct net_device *dev = priv->dev; > + struct ethhdr *phdr = (struct ethhdr *)(skb->data); > + char *src_addr = phdr->h_source; > + int i, slot = -1, oldest_slot = -1; > + unsigned long oldest_time = jiffies; > + > + if (ether_addr_equal(src_addr, dev->dev_addr) || > + !is_valid_ether_addr(src_addr)) > + return; > + > + for (i = 0; i < priv->max_macs; i++) { > + /* update timestamp if we already learned the address */ > + if (ether_addr_equal(priv->mac_list[i].mac_addr, src_addr)) { > + priv->mac_list[i].time = jiffies; > + return; > + } > + /* find empty slot */ > + if (slot >= 0) > + continue; > + if (!priv->mac_list[i].time) > + slot = i; > + else if (time_before(priv->mac_list[i].time, oldest_time)) { > + oldest_time = priv->mac_list[i].time; > + oldest_slot = i; > + } > + } > + if ((slot < 0) && (oldest_slot >= 0)) { > + slot = oldest_slot; > + priv->xstats.learning_drop++; > + dev_uc_del(dev, priv->mac_list[slot].mac_addr); > + } > + > + memcpy(priv->mac_list[slot].mac_addr, src_addr, ETH_ALEN); > + dev_uc_add_excl(dev, src_addr); > + priv->mac_list[slot].time = jiffies; > +} > + > /** > * xgmac_xmit: > * @skb : the socket buffer > @@ -1155,6 +1272,9 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev) > if (tx_dma_ring_space(priv) > MAX_SKB_FRAGS) > netif_start_queue(dev); > } > + > + xgmac_learn_mac(priv, skb); > + > return NETDEV_TX_OK; > > dma_err: > @@ -1291,6 +1411,8 @@ static void xgmac_set_rx_mode(struct net_device *dev) > netdev_dbg(priv->dev, "# mcasts %d, # unicast %d\n", > netdev_mc_count(dev), netdev_uc_count(dev)); > > + xgmac_check_mac_addrs(priv); > + > if (dev->flags & IFF_PROMISC) > value |= XGMAC_FRAME_FILTER_PR; > > @@ -1309,6 +1431,8 @@ static void xgmac_set_rx_mode(struct net_device *dev) > * within the register. */ > hash_filter[bit_nr >> 5] |= 1 << (bit_nr & 31); > } else { > + if (xgmac_mac_rm_pending(priv, ha->addr)) > + continue; > xgmac_set_mac_addr(ioaddr, ha->addr, reg); > reg++; > } > @@ -1605,6 +1729,7 @@ static const struct xgmac_stats xgmac_gstrings_stats[] = { > XGMAC_STAT(rx_ip_header_error), > XGMAC_STAT(rx_da_filter_fail), > XGMAC_STAT(fatal_bus_error), > + XGMAC_STAT(learning_drop), > XGMAC_HW_STAT(rx_watchdog, XGMAC_MMC_RXWATCHDOG), > XGMAC_HW_STAT(tx_vlan, XGMAC_MMC_TXVLANFRAME), > XGMAC_HW_STAT(rx_vlan, XGMAC_MMC_RXVLANFRAME), > @@ -1743,6 +1868,7 @@ static int xgmac_probe(struct platform_device *pdev) > SET_ETHTOOL_OPS(ndev, &xgmac_ethtool_ops); > spin_lock_init(&priv->stats_lock); > INIT_WORK(&priv->tx_timeout_work, xgmac_tx_timeout_work); > + INIT_DELAYED_WORK(&priv->mac_aging_work, xgmac_mac_aging_work); > > priv->device = &pdev->dev; > priv->dev = ndev; > Rob - I'm curious why you don't just always use the hardware hash filter table ? Is there a significant performance hit ? Using the hash filter table means that you don't have to worry about managing a slot array. You could use a much simpler linked list and just not worry about collisions or allocations in mac_list. Receive address filtering does not have to be perfect. rtg
On 11/15/2013 11:21 AM, Tim Gardner wrote: > On 11/14/2013 02:01 PM, Rob Herring wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> The Calxeda xgmac must learn secondary mac addresses such as those >> behind a bridge in order to properly receive packets with those mac >> addresses. Add mac address learning on transmit with timestamping of >> entries. The mac addresses are added to the driver's unicast address >> list, so they are visible to user via "bridge fdb show" command. >> >> Addresses can get disabled when the AE bit is cleared, so address >> registers must be checked for possible removal and update the unicast >> list. >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> --- > Rob - I'm curious why you don't just always use the hardware hash filter > table ? Is there a significant performance hit ? Using the hash filter > table means that you don't have to worry about managing a slot array. > You could use a much simpler linked list and just not worry about > collisions or allocations in mac_list. The fabric has to know every exact unicast address to route packets, so the hash filter doesn't really work. While the fabric is kind of switch like, there is no broadcast unknown addresses like an L2 switch. There is a lot of work the management core does here as addresses are added or removed here. Rob
On 11/15/2013 09:36 AM, Rob Herring wrote: > On 11/15/2013 11:21 AM, Tim Gardner wrote: >> On 11/14/2013 02:01 PM, Rob Herring wrote: >>> From: Rob Herring <rob.herring@calxeda.com> >>> >>> The Calxeda xgmac must learn secondary mac addresses such as those >>> behind a bridge in order to properly receive packets with those mac >>> addresses. Add mac address learning on transmit with timestamping of >>> entries. The mac addresses are added to the driver's unicast address >>> list, so they are visible to user via "bridge fdb show" command. >>> >>> Addresses can get disabled when the AE bit is cleared, so address >>> registers must be checked for possible removal and update the unicast >>> list. >>> >>> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >>> --- > >> Rob - I'm curious why you don't just always use the hardware hash filter >> table ? Is there a significant performance hit ? Using the hash filter >> table means that you don't have to worry about managing a slot array. >> You could use a much simpler linked list and just not worry about >> collisions or allocations in mac_list. > > The fabric has to know every exact unicast address to route packets, so > the hash filter doesn't really work. While the fabric is kind of switch > like, there is no broadcast unknown addresses like an L2 switch. There > is a lot of work the management core does here as addresses are added or > removed here. > > Rob > xgmac_set_rx_mode() falls back to the hash table method when the combined number of unicast and multicast addresses exceed the number of slots. What happens then ? rtg
More comments inline. On 11/14/2013 02:01 PM, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > The Calxeda xgmac must learn secondary mac addresses such as those > behind a bridge in order to properly receive packets with those mac > addresses. Add mac address learning on transmit with timestamping of > entries. The mac addresses are added to the driver's unicast address > list, so they are visible to user via "bridge fdb show" command. > > Addresses can get disabled when the AE bit is cleared, so address > registers must be checked for possible removal and update the unicast > list. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > --- > drivers/net/ethernet/calxeda/xgmac.c | 128 ++++++++++++++++++++++++++++++++++- > 1 file changed, 127 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c > index 48f5288..584aca8 100644 > --- a/drivers/net/ethernet/calxeda/xgmac.c > +++ b/drivers/net/ethernet/calxeda/xgmac.c > @@ -360,6 +360,13 @@ struct xgmac_extra_stats { > unsigned long rx_process_stopped; > unsigned long tx_early; > unsigned long fatal_bus_error; > + unsigned long learning_drop; > +}; > + > +struct xgmac_mac { > + char mac_addr[ETH_ALEN]; > + unsigned long time; > + bool rm_pending; > }; > > struct xgmac_priv { > @@ -384,6 +391,8 @@ struct xgmac_priv { > struct napi_struct napi; > > int max_macs; > + struct xgmac_mac mac_list[32]; > + > struct xgmac_extra_stats xstats; > > spinlock_t stats_lock; > @@ -392,6 +401,7 @@ struct xgmac_priv { > char tx_pause; > int wolopts; > struct work_struct tx_timeout_work; > + struct delayed_work mac_aging_work; > }; > > /* XGMAC Configuration Settings */ > @@ -634,7 +644,7 @@ static void xgmac_set_mac_addr(void __iomem *ioaddr, unsigned char *addr, > } > } > > -static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, > +static bool xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, > int num) > { > u32 hi_addr, lo_addr; > @@ -650,6 +660,8 @@ static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, > addr[3] = (lo_addr >> 24) & 0xff; > addr[4] = hi_addr & 0xff; > addr[5] = (hi_addr >> 8) & 0xff; > + > + return (hi_addr & XGMAC_ADDR_AE) ? true : false; > } > > static int xgmac_set_flow_ctrl(struct xgmac_priv *priv, int rx, int tx) > @@ -1047,6 +1059,8 @@ static int xgmac_open(struct net_device *dev) > writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS); > writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA); > > + schedule_delayed_work(&priv->mac_aging_work, 5 * HZ); > + > return 0; > } > > @@ -1059,6 +1073,7 @@ static int xgmac_open(struct net_device *dev) > static int xgmac_stop(struct net_device *dev) > { > struct xgmac_priv *priv = netdev_priv(dev); > + int i; > > netif_stop_queue(dev); > > @@ -1073,9 +1088,111 @@ static int xgmac_stop(struct net_device *dev) > /* Release and free the Rx/Tx resources */ > xgmac_free_dma_desc_rings(priv); > > + cancel_delayed_work_sync(&priv->mac_aging_work); > + for (i = 0; i < priv->max_macs; i++) { > + if (!is_valid_ether_addr(priv->mac_list[i].mac_addr)) > + continue; > + dev_uc_del(dev, priv->mac_list[i].mac_addr); > + memset(&priv->mac_list[i], 0, sizeof(priv->mac_list[i])); > + } > + > return 0; > } > > +static void xgmac_check_mac_addrs(struct xgmac_priv *priv) > +{ > + int i, j; > + char addr[ETH_ALEN]; > + > + for (i = 1; i < priv->max_macs; i++) { > + if (xgmac_get_mac_addr(priv->base, addr, i)) > + continue; > + > + if (!is_valid_ether_addr(addr)) > + break; > + > + for (j = 1; j < priv->max_macs; j++) { > + if (!ether_addr_equal(addr, priv->mac_list[j].mac_addr)) > + continue; > + priv->mac_list[j].rm_pending = true; > + break; > + } > + } > +} Shouldn't there be a test for how old a MAC address is before you mark it for removal ? Otherwise every address in priv->mac_list that is also in the HW table gets marked for removal each time this function is called. Are entries getting removed from the HW table somewhere else ? xgmac_check_mac_addrs() is also called from xgmac_set_rx_mode(). I just can't see how that is going to work when addition of an address to the HW table is predicated on xgmac_mac_rm_pending() being false. > + > +static bool xgmac_mac_rm_pending(struct xgmac_priv *priv, const u8 *addr) > +{ > + int i; > + > + for (i = 1; i < priv->max_macs; i++) { > + if (ether_addr_equal(addr, priv->mac_list[i].mac_addr) && > + priv->mac_list[i].rm_pending) > + return true; > + } > + return false; > +} > + > +static void xgmac_mac_aging_work(struct work_struct *work) > +{ > + int i; > + struct xgmac_priv *priv = > + container_of(work, struct xgmac_priv, mac_aging_work.work); > + struct net_device *dev = priv->dev; > + > + xgmac_check_mac_addrs(priv); > + > + for (i = 1; i < priv->max_macs; i++) { > + if (!priv->mac_list[i].rm_pending) > + continue; > + > + dev_uc_del(dev, priv->mac_list[i].mac_addr); > + priv->mac_list[i].rm_pending = false; > + priv->mac_list[i].time = 0; > + memset(priv->mac_list[i].mac_addr, 0, ETH_ALEN); > + } > + > + schedule_delayed_work(to_delayed_work(work), 10 * HZ); > +} > + > +static void xgmac_learn_mac(struct xgmac_priv *priv, struct sk_buff *skb) > +{ > + struct net_device *dev = priv->dev; > + struct ethhdr *phdr = (struct ethhdr *)(skb->data); > + char *src_addr = phdr->h_source; > + int i, slot = -1, oldest_slot = -1; > + unsigned long oldest_time = jiffies; > + > + if (ether_addr_equal(src_addr, dev->dev_addr) || > + !is_valid_ether_addr(src_addr)) > + return; > + > + for (i = 0; i < priv->max_macs; i++) { > + /* update timestamp if we already learned the address */ > + if (ether_addr_equal(priv->mac_list[i].mac_addr, src_addr)) { > + priv->mac_list[i].time = jiffies; > + return; > + } > + /* find empty slot */ > + if (slot >= 0) > + continue; > + if (!priv->mac_list[i].time) > + slot = i; > + else if (time_before(priv->mac_list[i].time, oldest_time)) { > + oldest_time = priv->mac_list[i].time; > + oldest_slot = i; > + } > + } > + if ((slot < 0) && (oldest_slot >= 0)) { > + slot = oldest_slot; > + priv->xstats.learning_drop++; > + dev_uc_del(dev, priv->mac_list[slot].mac_addr); > + } > + > + memcpy(priv->mac_list[slot].mac_addr, src_addr, ETH_ALEN); > + dev_uc_add_excl(dev, src_addr); > + priv->mac_list[slot].time = jiffies; > +} > + > /** > * xgmac_xmit: > * @skb : the socket buffer > @@ -1155,6 +1272,9 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev) > if (tx_dma_ring_space(priv) > MAX_SKB_FRAGS) > netif_start_queue(dev); > } > + > + xgmac_learn_mac(priv, skb); > + > return NETDEV_TX_OK; > > dma_err: > @@ -1291,6 +1411,8 @@ static void xgmac_set_rx_mode(struct net_device *dev) > netdev_dbg(priv->dev, "# mcasts %d, # unicast %d\n", > netdev_mc_count(dev), netdev_uc_count(dev)); > > + xgmac_check_mac_addrs(priv); > + > if (dev->flags & IFF_PROMISC) > value |= XGMAC_FRAME_FILTER_PR; > > @@ -1309,6 +1431,8 @@ static void xgmac_set_rx_mode(struct net_device *dev) > * within the register. */ > hash_filter[bit_nr >> 5] |= 1 << (bit_nr & 31); > } else { > + if (xgmac_mac_rm_pending(priv, ha->addr)) > + continue; > xgmac_set_mac_addr(ioaddr, ha->addr, reg); > reg++; > } > @@ -1605,6 +1729,7 @@ static const struct xgmac_stats xgmac_gstrings_stats[] = { > XGMAC_STAT(rx_ip_header_error), > XGMAC_STAT(rx_da_filter_fail), > XGMAC_STAT(fatal_bus_error), > + XGMAC_STAT(learning_drop), > XGMAC_HW_STAT(rx_watchdog, XGMAC_MMC_RXWATCHDOG), > XGMAC_HW_STAT(tx_vlan, XGMAC_MMC_TXVLANFRAME), > XGMAC_HW_STAT(rx_vlan, XGMAC_MMC_RXVLANFRAME), > @@ -1743,6 +1868,7 @@ static int xgmac_probe(struct platform_device *pdev) > SET_ETHTOOL_OPS(ndev, &xgmac_ethtool_ops); > spin_lock_init(&priv->stats_lock); > INIT_WORK(&priv->tx_timeout_work, xgmac_tx_timeout_work); > + INIT_DELAYED_WORK(&priv->mac_aging_work, xgmac_mac_aging_work); > > priv->device = &pdev->dev; > priv->dev = ndev; >
On 11/15/2013 01:54 PM, Tim Gardner wrote: > More comments inline. > > On 11/14/2013 02:01 PM, Rob Herring wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> The Calxeda xgmac must learn secondary mac addresses such as those >> behind a bridge in order to properly receive packets with those mac >> addresses. Add mac address learning on transmit with timestamping of >> entries. The mac addresses are added to the driver's unicast address >> list, so they are visible to user via "bridge fdb show" command. >> >> Addresses can get disabled when the AE bit is cleared, so address >> registers must be checked for possible removal and update the unicast >> list. >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> --- >> drivers/net/ethernet/calxeda/xgmac.c | 128 ++++++++++++++++++++++++++++++++++- >> 1 file changed, 127 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c >> index 48f5288..584aca8 100644 >> --- a/drivers/net/ethernet/calxeda/xgmac.c >> +++ b/drivers/net/ethernet/calxeda/xgmac.c >> @@ -360,6 +360,13 @@ struct xgmac_extra_stats { >> unsigned long rx_process_stopped; >> unsigned long tx_early; >> unsigned long fatal_bus_error; >> + unsigned long learning_drop; >> +}; >> + >> +struct xgmac_mac { >> + char mac_addr[ETH_ALEN]; >> + unsigned long time; >> + bool rm_pending; >> }; >> >> struct xgmac_priv { >> @@ -384,6 +391,8 @@ struct xgmac_priv { >> struct napi_struct napi; >> >> int max_macs; >> + struct xgmac_mac mac_list[32]; >> + >> struct xgmac_extra_stats xstats; >> >> spinlock_t stats_lock; >> @@ -392,6 +401,7 @@ struct xgmac_priv { >> char tx_pause; >> int wolopts; >> struct work_struct tx_timeout_work; >> + struct delayed_work mac_aging_work; >> }; >> >> /* XGMAC Configuration Settings */ >> @@ -634,7 +644,7 @@ static void xgmac_set_mac_addr(void __iomem *ioaddr, unsigned char *addr, >> } >> } >> >> -static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >> +static bool xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >> int num) >> { >> u32 hi_addr, lo_addr; >> @@ -650,6 +660,8 @@ static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >> addr[3] = (lo_addr >> 24) & 0xff; >> addr[4] = hi_addr & 0xff; >> addr[5] = (hi_addr >> 8) & 0xff; >> + >> + return (hi_addr & XGMAC_ADDR_AE) ? true : false; >> } >> >> static int xgmac_set_flow_ctrl(struct xgmac_priv *priv, int rx, int tx) >> @@ -1047,6 +1059,8 @@ static int xgmac_open(struct net_device *dev) >> writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS); >> writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA); >> >> + schedule_delayed_work(&priv->mac_aging_work, 5 * HZ); >> + >> return 0; >> } >> >> @@ -1059,6 +1073,7 @@ static int xgmac_open(struct net_device *dev) >> static int xgmac_stop(struct net_device *dev) >> { >> struct xgmac_priv *priv = netdev_priv(dev); >> + int i; >> >> netif_stop_queue(dev); >> >> @@ -1073,9 +1088,111 @@ static int xgmac_stop(struct net_device *dev) >> /* Release and free the Rx/Tx resources */ >> xgmac_free_dma_desc_rings(priv); >> >> + cancel_delayed_work_sync(&priv->mac_aging_work); >> + for (i = 0; i < priv->max_macs; i++) { >> + if (!is_valid_ether_addr(priv->mac_list[i].mac_addr)) >> + continue; >> + dev_uc_del(dev, priv->mac_list[i].mac_addr); >> + memset(&priv->mac_list[i], 0, sizeof(priv->mac_list[i])); >> + } >> + >> return 0; >> } >> >> +static void xgmac_check_mac_addrs(struct xgmac_priv *priv) >> +{ >> + int i, j; >> + char addr[ETH_ALEN]; >> + >> + for (i = 1; i < priv->max_macs; i++) { >> + if (xgmac_get_mac_addr(priv->base, addr, i)) >> + continue; >> + >> + if (!is_valid_ether_addr(addr)) >> + break; >> + >> + for (j = 1; j < priv->max_macs; j++) { >> + if (!ether_addr_equal(addr, priv->mac_list[j].mac_addr)) >> + continue; >> + priv->mac_list[j].rm_pending = true; >> + break; >> + } >> + } >> +} > > Shouldn't there be a test for how old a MAC address is before you mark > it for removal ? Otherwise every address in priv->mac_list that is also > in the HW table gets marked for removal each time this function is > called. Are entries getting removed from the HW table somewhere else ? Yes. There is a valid bit which can be cleared by the firmware and we have to check this before changing the MAC registers. This would only happen if the same address shows up on another node which is the case for VM migration (which isn't really fully baked on arm yet). > xgmac_check_mac_addrs() is also called from xgmac_set_rx_mode(). I just > can't see how that is going to work when addition of an address to the > HW table is predicated on xgmac_mac_rm_pending() being false. The flow here is a bit weird because I can't call dev_uc_del within xgmac_set_rx_mode as the list lock is already held. So we have this intermediate state where the address needs to be removed but is still in the unicast list. The check is simply to avoid re-adding an address. Rob
On 11/15/2013 12:00 PM, Tim Gardner wrote: > On 11/15/2013 09:36 AM, Rob Herring wrote: >> On 11/15/2013 11:21 AM, Tim Gardner wrote: >>> On 11/14/2013 02:01 PM, Rob Herring wrote: >>>> From: Rob Herring <rob.herring@calxeda.com> >>>> >>>> The Calxeda xgmac must learn secondary mac addresses such as those >>>> behind a bridge in order to properly receive packets with those mac >>>> addresses. Add mac address learning on transmit with timestamping of >>>> entries. The mac addresses are added to the driver's unicast address >>>> list, so they are visible to user via "bridge fdb show" command. >>>> >>>> Addresses can get disabled when the AE bit is cleared, so address >>>> registers must be checked for possible removal and update the unicast >>>> list. >>>> >>>> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >>>> --- >> >>> Rob - I'm curious why you don't just always use the hardware hash filter >>> table ? Is there a significant performance hit ? Using the hash filter >>> table means that you don't have to worry about managing a slot array. >>> You could use a much simpler linked list and just not worry about >>> collisions or allocations in mac_list. >> >> The fabric has to know every exact unicast address to route packets, so >> the hash filter doesn't really work. While the fabric is kind of switch >> like, there is no broadcast unknown addresses like an L2 switch. There >> is a lot of work the management core does here as addresses are added or >> removed here. >> >> Rob >> > > xgmac_set_rx_mode() falls back to the hash table method when the > combined number of unicast and multicast addresses exceed the number of > slots. What happens then ? We will use all slots for unicast first and then use hash filter if there are not enough slots left. But yes, if you exceed 31 unicast addresses things will stop working. We should fix this for the "bridge fdb add" case, but failure won't be so noticeable in the learning case. In the learning case, the only way I've come up with to report errors is incrementing the learning_drop counter or a printk. Rob
On 11/15/2013 02:32 PM, Rob Herring wrote: > On 11/15/2013 01:54 PM, Tim Gardner wrote: >> More comments inline. >> >> On 11/14/2013 02:01 PM, Rob Herring wrote: >>> From: Rob Herring <rob.herring@calxeda.com> >>> >>> The Calxeda xgmac must learn secondary mac addresses such as those >>> behind a bridge in order to properly receive packets with those mac >>> addresses. Add mac address learning on transmit with timestamping of >>> entries. The mac addresses are added to the driver's unicast address >>> list, so they are visible to user via "bridge fdb show" command. >>> >>> Addresses can get disabled when the AE bit is cleared, so address >>> registers must be checked for possible removal and update the unicast >>> list. >>> >>> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >>> --- >>> drivers/net/ethernet/calxeda/xgmac.c | 128 ++++++++++++++++++++++++++++++++++- >>> 1 file changed, 127 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c >>> index 48f5288..584aca8 100644 >>> --- a/drivers/net/ethernet/calxeda/xgmac.c >>> +++ b/drivers/net/ethernet/calxeda/xgmac.c >>> @@ -360,6 +360,13 @@ struct xgmac_extra_stats { >>> unsigned long rx_process_stopped; >>> unsigned long tx_early; >>> unsigned long fatal_bus_error; >>> + unsigned long learning_drop; >>> +}; >>> + >>> +struct xgmac_mac { >>> + char mac_addr[ETH_ALEN]; >>> + unsigned long time; >>> + bool rm_pending; >>> }; >>> >>> struct xgmac_priv { >>> @@ -384,6 +391,8 @@ struct xgmac_priv { >>> struct napi_struct napi; >>> >>> int max_macs; >>> + struct xgmac_mac mac_list[32]; >>> + >>> struct xgmac_extra_stats xstats; >>> >>> spinlock_t stats_lock; >>> @@ -392,6 +401,7 @@ struct xgmac_priv { >>> char tx_pause; >>> int wolopts; >>> struct work_struct tx_timeout_work; >>> + struct delayed_work mac_aging_work; >>> }; >>> >>> /* XGMAC Configuration Settings */ >>> @@ -634,7 +644,7 @@ static void xgmac_set_mac_addr(void __iomem *ioaddr, unsigned char *addr, >>> } >>> } >>> >>> -static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >>> +static bool xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >>> int num) >>> { >>> u32 hi_addr, lo_addr; >>> @@ -650,6 +660,8 @@ static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >>> addr[3] = (lo_addr >> 24) & 0xff; >>> addr[4] = hi_addr & 0xff; >>> addr[5] = (hi_addr >> 8) & 0xff; >>> + >>> + return (hi_addr & XGMAC_ADDR_AE) ? true : false; >>> } >>> >>> static int xgmac_set_flow_ctrl(struct xgmac_priv *priv, int rx, int tx) >>> @@ -1047,6 +1059,8 @@ static int xgmac_open(struct net_device *dev) >>> writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS); >>> writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA); >>> >>> + schedule_delayed_work(&priv->mac_aging_work, 5 * HZ); >>> + >>> return 0; >>> } >>> >>> @@ -1059,6 +1073,7 @@ static int xgmac_open(struct net_device *dev) >>> static int xgmac_stop(struct net_device *dev) >>> { >>> struct xgmac_priv *priv = netdev_priv(dev); >>> + int i; >>> >>> netif_stop_queue(dev); >>> >>> @@ -1073,9 +1088,111 @@ static int xgmac_stop(struct net_device *dev) >>> /* Release and free the Rx/Tx resources */ >>> xgmac_free_dma_desc_rings(priv); >>> >>> + cancel_delayed_work_sync(&priv->mac_aging_work); >>> + for (i = 0; i < priv->max_macs; i++) { >>> + if (!is_valid_ether_addr(priv->mac_list[i].mac_addr)) >>> + continue; >>> + dev_uc_del(dev, priv->mac_list[i].mac_addr); >>> + memset(&priv->mac_list[i], 0, sizeof(priv->mac_list[i])); >>> + } >>> + >>> return 0; >>> } >>> >>> +static void xgmac_check_mac_addrs(struct xgmac_priv *priv) >>> +{ >>> + int i, j; >>> + char addr[ETH_ALEN]; >>> + >>> + for (i = 1; i < priv->max_macs; i++) { >>> + if (xgmac_get_mac_addr(priv->base, addr, i)) >>> + continue; >>> + >>> + if (!is_valid_ether_addr(addr)) >>> + break; >>> + >>> + for (j = 1; j < priv->max_macs; j++) { >>> + if (!ether_addr_equal(addr, priv->mac_list[j].mac_addr)) >>> + continue; >>> + priv->mac_list[j].rm_pending = true; >>> + break; >>> + } >>> + } >>> +} >> >> Shouldn't there be a test for how old a MAC address is before you mark >> it for removal ? Otherwise every address in priv->mac_list that is also >> in the HW table gets marked for removal each time this function is >> called. Are entries getting removed from the HW table somewhere else ? > > Yes. There is a valid bit which can be cleared by the firmware and we > have to check this before changing the MAC registers. This would only > happen if the same address shows up on another node which is the case > for VM migration (which isn't really fully baked on arm yet). > >> xgmac_check_mac_addrs() is also called from xgmac_set_rx_mode(). I just >> can't see how that is going to work when addition of an address to the >> HW table is predicated on xgmac_mac_rm_pending() being false. > > The flow here is a bit weird because I can't call dev_uc_del within > xgmac_set_rx_mode as the list lock is already held. So we have this > intermediate state where the address needs to be removed but is still in > the unicast list. The check is simply to avoid re-adding an address. > > Rob > Well, I still think this doesn't do you intend. How about adding some debug so we can see when these MAC addresses come and go ? If debug is flooding the log then we'll know something is wrong 'cause it should only happen when you migrate a VM. I wonder if there is already a general way of viewing MAC addresses that have been entered into the dev MAC table via dev_uc_add()/dev_uc_del() ? rtg
On 11/19/2013 08:42 AM, Tim Gardner wrote: > On 11/15/2013 02:32 PM, Rob Herring wrote: >> On 11/15/2013 01:54 PM, Tim Gardner wrote: >>> More comments inline. >>> >>> On 11/14/2013 02:01 PM, Rob Herring wrote: >>>> From: Rob Herring <rob.herring@calxeda.com> >>>> >>>> The Calxeda xgmac must learn secondary mac addresses such as those >>>> behind a bridge in order to properly receive packets with those mac >>>> addresses. Add mac address learning on transmit with timestamping of >>>> entries. The mac addresses are added to the driver's unicast address >>>> list, so they are visible to user via "bridge fdb show" command. >>>> >>>> Addresses can get disabled when the AE bit is cleared, so address >>>> registers must be checked for possible removal and update the unicast >>>> list. >>>> >>>> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >>>> --- >>>> drivers/net/ethernet/calxeda/xgmac.c | 128 ++++++++++++++++++++++++++++++++++- >>>> 1 file changed, 127 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c >>>> index 48f5288..584aca8 100644 >>>> --- a/drivers/net/ethernet/calxeda/xgmac.c >>>> +++ b/drivers/net/ethernet/calxeda/xgmac.c >>>> @@ -360,6 +360,13 @@ struct xgmac_extra_stats { >>>> unsigned long rx_process_stopped; >>>> unsigned long tx_early; >>>> unsigned long fatal_bus_error; >>>> + unsigned long learning_drop; >>>> +}; >>>> + >>>> +struct xgmac_mac { >>>> + char mac_addr[ETH_ALEN]; >>>> + unsigned long time; >>>> + bool rm_pending; >>>> }; >>>> >>>> struct xgmac_priv { >>>> @@ -384,6 +391,8 @@ struct xgmac_priv { >>>> struct napi_struct napi; >>>> >>>> int max_macs; >>>> + struct xgmac_mac mac_list[32]; >>>> + >>>> struct xgmac_extra_stats xstats; >>>> >>>> spinlock_t stats_lock; >>>> @@ -392,6 +401,7 @@ struct xgmac_priv { >>>> char tx_pause; >>>> int wolopts; >>>> struct work_struct tx_timeout_work; >>>> + struct delayed_work mac_aging_work; >>>> }; >>>> >>>> /* XGMAC Configuration Settings */ >>>> @@ -634,7 +644,7 @@ static void xgmac_set_mac_addr(void __iomem *ioaddr, unsigned char *addr, >>>> } >>>> } >>>> >>>> -static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >>>> +static bool xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >>>> int num) >>>> { >>>> u32 hi_addr, lo_addr; >>>> @@ -650,6 +660,8 @@ static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, >>>> addr[3] = (lo_addr >> 24) & 0xff; >>>> addr[4] = hi_addr & 0xff; >>>> addr[5] = (hi_addr >> 8) & 0xff; >>>> + >>>> + return (hi_addr & XGMAC_ADDR_AE) ? true : false; >>>> } >>>> >>>> static int xgmac_set_flow_ctrl(struct xgmac_priv *priv, int rx, int tx) >>>> @@ -1047,6 +1059,8 @@ static int xgmac_open(struct net_device *dev) >>>> writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS); >>>> writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA); >>>> >>>> + schedule_delayed_work(&priv->mac_aging_work, 5 * HZ); >>>> + >>>> return 0; >>>> } >>>> >>>> @@ -1059,6 +1073,7 @@ static int xgmac_open(struct net_device *dev) >>>> static int xgmac_stop(struct net_device *dev) >>>> { >>>> struct xgmac_priv *priv = netdev_priv(dev); >>>> + int i; >>>> >>>> netif_stop_queue(dev); >>>> >>>> @@ -1073,9 +1088,111 @@ static int xgmac_stop(struct net_device *dev) >>>> /* Release and free the Rx/Tx resources */ >>>> xgmac_free_dma_desc_rings(priv); >>>> >>>> + cancel_delayed_work_sync(&priv->mac_aging_work); >>>> + for (i = 0; i < priv->max_macs; i++) { >>>> + if (!is_valid_ether_addr(priv->mac_list[i].mac_addr)) >>>> + continue; >>>> + dev_uc_del(dev, priv->mac_list[i].mac_addr); >>>> + memset(&priv->mac_list[i], 0, sizeof(priv->mac_list[i])); >>>> + } >>>> + >>>> return 0; >>>> } >>>> >>>> +static void xgmac_check_mac_addrs(struct xgmac_priv *priv) >>>> +{ >>>> + int i, j; >>>> + char addr[ETH_ALEN]; >>>> + >>>> + for (i = 1; i < priv->max_macs; i++) { >>>> + if (xgmac_get_mac_addr(priv->base, addr, i)) >>>> + continue; >>>> + >>>> + if (!is_valid_ether_addr(addr)) >>>> + break; >>>> + >>>> + for (j = 1; j < priv->max_macs; j++) { >>>> + if (!ether_addr_equal(addr, priv->mac_list[j].mac_addr)) >>>> + continue; >>>> + priv->mac_list[j].rm_pending = true; >>>> + break; >>>> + } >>>> + } >>>> +} >>> >>> Shouldn't there be a test for how old a MAC address is before you mark >>> it for removal ? Otherwise every address in priv->mac_list that is also >>> in the HW table gets marked for removal each time this function is >>> called. Are entries getting removed from the HW table somewhere else ? >> >> Yes. There is a valid bit which can be cleared by the firmware and we >> have to check this before changing the MAC registers. This would only >> happen if the same address shows up on another node which is the case >> for VM migration (which isn't really fully baked on arm yet). >> >>> xgmac_check_mac_addrs() is also called from xgmac_set_rx_mode(). I just >>> can't see how that is going to work when addition of an address to the >>> HW table is predicated on xgmac_mac_rm_pending() being false. >> >> The flow here is a bit weird because I can't call dev_uc_del within >> xgmac_set_rx_mode as the list lock is already held. So we have this >> intermediate state where the address needs to be removed but is still in >> the unicast list. The check is simply to avoid re-adding an address. >> >> Rob >> > > Well, I still think this doesn't do you intend. How about adding some > debug so we can see when these MAC addresses come and go ? If debug is > flooding the log then we'll know something is wrong 'cause it should > only happen when you migrate a VM. > > I wonder if there is already a general way of viewing MAC addresses that > have been entered into the dev MAC table via dev_uc_add()/dev_uc_del() ? Yes, "bridge fdb show" shows the entries in the list. I use pktgen to generate packets with new source MAC addresses. Then checking with "bridge fdb show" I can see they are added. Then with devmem2 I can check the h/w registers directly. Also with devmem2, I can clear the AE bit to trigger a removal manually and verify with "bridge fdb show". Rob
On Thu, Nov 14, 2013 at 04:01:59PM -0600, Rob Herring wrote: > From: Rob Herring <rob.herring@calxeda.com> > > The Calxeda xgmac must learn secondary mac addresses such as those > behind a bridge in order to properly receive packets with those mac > addresses. Add mac address learning on transmit with timestamping of > entries. The mac addresses are added to the driver's unicast address > list, so they are visible to user via "bridge fdb show" command. > > Addresses can get disabled when the AE bit is cleared, so address > registers must be checked for possible removal and update the unicast > list. > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> This looks to fix the issues I had with the previous version. Therefore: Acked-by: Andy Whitcroft <apw@canonical.com> -apw
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c index 48f5288..584aca8 100644 --- a/drivers/net/ethernet/calxeda/xgmac.c +++ b/drivers/net/ethernet/calxeda/xgmac.c @@ -360,6 +360,13 @@ struct xgmac_extra_stats { unsigned long rx_process_stopped; unsigned long tx_early; unsigned long fatal_bus_error; + unsigned long learning_drop; +}; + +struct xgmac_mac { + char mac_addr[ETH_ALEN]; + unsigned long time; + bool rm_pending; }; struct xgmac_priv { @@ -384,6 +391,8 @@ struct xgmac_priv { struct napi_struct napi; int max_macs; + struct xgmac_mac mac_list[32]; + struct xgmac_extra_stats xstats; spinlock_t stats_lock; @@ -392,6 +401,7 @@ struct xgmac_priv { char tx_pause; int wolopts; struct work_struct tx_timeout_work; + struct delayed_work mac_aging_work; }; /* XGMAC Configuration Settings */ @@ -634,7 +644,7 @@ static void xgmac_set_mac_addr(void __iomem *ioaddr, unsigned char *addr, } } -static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, +static bool xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, int num) { u32 hi_addr, lo_addr; @@ -650,6 +660,8 @@ static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr, addr[3] = (lo_addr >> 24) & 0xff; addr[4] = hi_addr & 0xff; addr[5] = (hi_addr >> 8) & 0xff; + + return (hi_addr & XGMAC_ADDR_AE) ? true : false; } static int xgmac_set_flow_ctrl(struct xgmac_priv *priv, int rx, int tx) @@ -1047,6 +1059,8 @@ static int xgmac_open(struct net_device *dev) writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS); writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA); + schedule_delayed_work(&priv->mac_aging_work, 5 * HZ); + return 0; } @@ -1059,6 +1073,7 @@ static int xgmac_open(struct net_device *dev) static int xgmac_stop(struct net_device *dev) { struct xgmac_priv *priv = netdev_priv(dev); + int i; netif_stop_queue(dev); @@ -1073,9 +1088,111 @@ static int xgmac_stop(struct net_device *dev) /* Release and free the Rx/Tx resources */ xgmac_free_dma_desc_rings(priv); + cancel_delayed_work_sync(&priv->mac_aging_work); + for (i = 0; i < priv->max_macs; i++) { + if (!is_valid_ether_addr(priv->mac_list[i].mac_addr)) + continue; + dev_uc_del(dev, priv->mac_list[i].mac_addr); + memset(&priv->mac_list[i], 0, sizeof(priv->mac_list[i])); + } + return 0; } +static void xgmac_check_mac_addrs(struct xgmac_priv *priv) +{ + int i, j; + char addr[ETH_ALEN]; + + for (i = 1; i < priv->max_macs; i++) { + if (xgmac_get_mac_addr(priv->base, addr, i)) + continue; + + if (!is_valid_ether_addr(addr)) + break; + + for (j = 1; j < priv->max_macs; j++) { + if (!ether_addr_equal(addr, priv->mac_list[j].mac_addr)) + continue; + priv->mac_list[j].rm_pending = true; + break; + } + } +} + +static bool xgmac_mac_rm_pending(struct xgmac_priv *priv, const u8 *addr) +{ + int i; + + for (i = 1; i < priv->max_macs; i++) { + if (ether_addr_equal(addr, priv->mac_list[i].mac_addr) && + priv->mac_list[i].rm_pending) + return true; + } + return false; +} + +static void xgmac_mac_aging_work(struct work_struct *work) +{ + int i; + struct xgmac_priv *priv = + container_of(work, struct xgmac_priv, mac_aging_work.work); + struct net_device *dev = priv->dev; + + xgmac_check_mac_addrs(priv); + + for (i = 1; i < priv->max_macs; i++) { + if (!priv->mac_list[i].rm_pending) + continue; + + dev_uc_del(dev, priv->mac_list[i].mac_addr); + priv->mac_list[i].rm_pending = false; + priv->mac_list[i].time = 0; + memset(priv->mac_list[i].mac_addr, 0, ETH_ALEN); + } + + schedule_delayed_work(to_delayed_work(work), 10 * HZ); +} + +static void xgmac_learn_mac(struct xgmac_priv *priv, struct sk_buff *skb) +{ + struct net_device *dev = priv->dev; + struct ethhdr *phdr = (struct ethhdr *)(skb->data); + char *src_addr = phdr->h_source; + int i, slot = -1, oldest_slot = -1; + unsigned long oldest_time = jiffies; + + if (ether_addr_equal(src_addr, dev->dev_addr) || + !is_valid_ether_addr(src_addr)) + return; + + for (i = 0; i < priv->max_macs; i++) { + /* update timestamp if we already learned the address */ + if (ether_addr_equal(priv->mac_list[i].mac_addr, src_addr)) { + priv->mac_list[i].time = jiffies; + return; + } + /* find empty slot */ + if (slot >= 0) + continue; + if (!priv->mac_list[i].time) + slot = i; + else if (time_before(priv->mac_list[i].time, oldest_time)) { + oldest_time = priv->mac_list[i].time; + oldest_slot = i; + } + } + if ((slot < 0) && (oldest_slot >= 0)) { + slot = oldest_slot; + priv->xstats.learning_drop++; + dev_uc_del(dev, priv->mac_list[slot].mac_addr); + } + + memcpy(priv->mac_list[slot].mac_addr, src_addr, ETH_ALEN); + dev_uc_add_excl(dev, src_addr); + priv->mac_list[slot].time = jiffies; +} + /** * xgmac_xmit: * @skb : the socket buffer @@ -1155,6 +1272,9 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev) if (tx_dma_ring_space(priv) > MAX_SKB_FRAGS) netif_start_queue(dev); } + + xgmac_learn_mac(priv, skb); + return NETDEV_TX_OK; dma_err: @@ -1291,6 +1411,8 @@ static void xgmac_set_rx_mode(struct net_device *dev) netdev_dbg(priv->dev, "# mcasts %d, # unicast %d\n", netdev_mc_count(dev), netdev_uc_count(dev)); + xgmac_check_mac_addrs(priv); + if (dev->flags & IFF_PROMISC) value |= XGMAC_FRAME_FILTER_PR; @@ -1309,6 +1431,8 @@ static void xgmac_set_rx_mode(struct net_device *dev) * within the register. */ hash_filter[bit_nr >> 5] |= 1 << (bit_nr & 31); } else { + if (xgmac_mac_rm_pending(priv, ha->addr)) + continue; xgmac_set_mac_addr(ioaddr, ha->addr, reg); reg++; } @@ -1605,6 +1729,7 @@ static const struct xgmac_stats xgmac_gstrings_stats[] = { XGMAC_STAT(rx_ip_header_error), XGMAC_STAT(rx_da_filter_fail), XGMAC_STAT(fatal_bus_error), + XGMAC_STAT(learning_drop), XGMAC_HW_STAT(rx_watchdog, XGMAC_MMC_RXWATCHDOG), XGMAC_HW_STAT(tx_vlan, XGMAC_MMC_TXVLANFRAME), XGMAC_HW_STAT(rx_vlan, XGMAC_MMC_RXVLANFRAME), @@ -1743,6 +1868,7 @@ static int xgmac_probe(struct platform_device *pdev) SET_ETHTOOL_OPS(ndev, &xgmac_ethtool_ops); spin_lock_init(&priv->stats_lock); INIT_WORK(&priv->tx_timeout_work, xgmac_tx_timeout_work); + INIT_DELAYED_WORK(&priv->mac_aging_work, xgmac_mac_aging_work); priv->device = &pdev->dev; priv->dev = ndev;