| Message ID | 1401969663-4464-2-git-send-email-dingtianhong@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 06/05/2014 08:01 AM, Ding Tianhong wrote: > If lowerdev supports L2 forwarding offload, no need to set mac address > to uc list and vlan list, so also don't do that when the macvlan mac address > changes. Same issue as in v1. Please explain how this actually works, since the new address never makes it to the hw. -vlad > > Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> > --- > drivers/net/macvlan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c > index 453d55a..c3a54a6 100644 > --- a/drivers/net/macvlan.c > +++ b/drivers/net/macvlan.c > @@ -515,7 +515,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) > struct net_device *lowerdev = vlan->lowerdev; > int err; > > - if (!(dev->flags & IFF_UP)) { > + if (!(dev->flags & IFF_UP) || vlan->fwd_priv) { > /* Just copy in the new address */ > ether_addr_copy(dev->dev_addr, addr); > } else { > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/6/5 23:09, Vlad Yasevich wrote: > On 06/05/2014 08:01 AM, Ding Tianhong wrote: >> If lowerdev supports L2 forwarding offload, no need to set mac address >> to uc list and vlan list, so also don't do that when the macvlan mac address >> changes. > > Same issue as in v1. Please explain how this actually works, since the > new address never makes it to the hw. > > -vlad >> Sorry, I miss the first feedback of yours, so didn't fix the issue from v1, I will think about it and than answer it, thanks. Ding >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >> --- >> drivers/net/macvlan.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c >> index 453d55a..c3a54a6 100644 >> --- a/drivers/net/macvlan.c >> +++ b/drivers/net/macvlan.c >> @@ -515,7 +515,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) >> struct net_device *lowerdev = vlan->lowerdev; >> int err; >> >> - if (!(dev->flags & IFF_UP)) { >> + if (!(dev->flags & IFF_UP) || vlan->fwd_priv) { >> /* Just copy in the new address */ >> ether_addr_copy(dev->dev_addr, addr); >> } else { >> > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/6/5 23:09, Vlad Yasevich wrote: > On 06/05/2014 08:01 AM, Ding Tianhong wrote: >> If lowerdev supports L2 forwarding offload, no need to set mac address >> to uc list and vlan list, so also don't do that when the macvlan mac address >> changes. > > Same issue as in v1. Please explain how this actually works, since the > new address never makes it to the hw. > Hi, vlad: The fwd only support 82599 nic, and when fwd_priv is true, this means the lowerdev support L2 forwarding offload, and will set the macvlan to the ixgbe_fwd_adapter->netdev, and then it is clear that the lowerdev could know that the the upper device is macvlan and could handle the skb to the upperdev. If I miss something, please remind me, thanks. Ding > -vlad >> >> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >> --- >> drivers/net/macvlan.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c >> index 453d55a..c3a54a6 100644 >> --- a/drivers/net/macvlan.c >> +++ b/drivers/net/macvlan.c >> @@ -515,7 +515,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) >> struct net_device *lowerdev = vlan->lowerdev; >> int err; >> >> - if (!(dev->flags & IFF_UP)) { >> + if (!(dev->flags & IFF_UP) || vlan->fwd_priv) { >> /* Just copy in the new address */ >> ether_addr_copy(dev->dev_addr, addr); >> } else { >> > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/05/2014 11:45 PM, Ding Tianhong wrote: > On 2014/6/5 23:09, Vlad Yasevich wrote: >> On 06/05/2014 08:01 AM, Ding Tianhong wrote: >>> If lowerdev supports L2 forwarding offload, no need to set mac address >>> to uc list and vlan list, so also don't do that when the macvlan mac address >>> changes. >> >> Same issue as in v1. Please explain how this actually works, since the >> new address never makes it to the hw. >> > > Hi, vlad: > > The fwd only support 82599 nic, and when fwd_priv is true, this means the lowerdev support L2 forwarding offload, and will set the macvlan > to the ixgbe_fwd_adapter->netdev, and then it is clear that the lowerdev could know that the the upper device is macvlan and could handle > the skb to the upperdev. > > If I miss something, please remind me, thanks. You've just changed the mac address of the macvlan device, but the address is not written to the card's RAR registers. So what allows the card to receive the traffic and place it into the queue in the first place? I can see it possibly working if the card is in promiscuous mode. Also, I think John is right in that if we simply do dev_uc_add() to add the address to the card, we'll unmap the address from the vmdqs and lose the acceleration. This needs more thought. -vlad > > Ding > >> -vlad >>> >>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >>> --- >>> drivers/net/macvlan.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c >>> index 453d55a..c3a54a6 100644 >>> --- a/drivers/net/macvlan.c >>> +++ b/drivers/net/macvlan.c >>> @@ -515,7 +515,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) >>> struct net_device *lowerdev = vlan->lowerdev; >>> int err; >>> >>> - if (!(dev->flags & IFF_UP)) { >>> + if (!(dev->flags & IFF_UP) || vlan->fwd_priv) { >>> /* Just copy in the new address */ >>> ether_addr_copy(dev->dev_addr, addr); >>> } else { >>> >> >> >> > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014/6/7 1:11, Vlad Yasevich wrote: > On 06/05/2014 11:45 PM, Ding Tianhong wrote: >> On 2014/6/5 23:09, Vlad Yasevich wrote: >>> On 06/05/2014 08:01 AM, Ding Tianhong wrote: >>>> If lowerdev supports L2 forwarding offload, no need to set mac address >>>> to uc list and vlan list, so also don't do that when the macvlan mac address >>>> changes. >>> >>> Same issue as in v1. Please explain how this actually works, since the >>> new address never makes it to the hw. >>> >> >> Hi, vlad: >> >> The fwd only support 82599 nic, and when fwd_priv is true, this means the lowerdev support L2 forwarding offload, and will set the macvlan >> to the ixgbe_fwd_adapter->netdev, and then it is clear that the lowerdev could know that the the upper device is macvlan and could handle >> the skb to the upperdev. >> >> If I miss something, please remind me, thanks. > > You've just changed the mac address of the macvlan device, but the > address is not written to the card's RAR registers. > So what allows the card to receive the traffic and place it into the > queue in the first place? > > I can see it possibly working if the card is in promiscuous mode. > > Also, I think John is right in that if we simply do dev_uc_add() > to add the address to the card, we'll unmap the address from the vmdqs > and lose the acceleration. This needs more thought. > Yes, I miss it, the new mac address should be set to the lowerdev by ndo_dfwd_add_station(), otherwise the lowerdev still use the old mac address and the macvlan could not work again, so I need to rethink this patch and resend, thanks, vlad. Ding > -vlad > >> >> Ding >> >>> -vlad >>>> >>>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> >>>> --- >>>> drivers/net/macvlan.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c >>>> index 453d55a..c3a54a6 100644 >>>> --- a/drivers/net/macvlan.c >>>> +++ b/drivers/net/macvlan.c >>>> @@ -515,7 +515,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) >>>> struct net_device *lowerdev = vlan->lowerdev; >>>> int err; >>>> >>>> - if (!(dev->flags & IFF_UP)) { >>>> + if (!(dev->flags & IFF_UP) || vlan->fwd_priv) { >>>> /* Just copy in the new address */ >>>> ether_addr_copy(dev->dev_addr, addr); >>>> } else { >>>> >>> >>> >>> >> >> > > > . > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c index 453d55a..c3a54a6 100644 --- a/drivers/net/macvlan.c +++ b/drivers/net/macvlan.c @@ -515,7 +515,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) struct net_device *lowerdev = vlan->lowerdev; int err; - if (!(dev->flags & IFF_UP)) { + if (!(dev->flags & IFF_UP) || vlan->fwd_priv) { /* Just copy in the new address */ ether_addr_copy(dev->dev_addr, addr); } else {
If lowerdev supports L2 forwarding offload, no need to set mac address to uc list and vlan list, so also don't do that when the macvlan mac address changes. Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> --- drivers/net/macvlan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)