On Wed, Feb 27, 2019 at 08:13:34PM -0800, Florian Fainelli wrote:
On 2/26/2019 10:45 AM, Ivan Khoronzhuk wrote:
The vlan device address is held separately from uc/mc lists and
handled differently. The vlan dev address is bound with real device
address only if it's inherited from init, in all other cases it's
separate address entry in uc list. With vid set, the address becomes
not inherited from real device after it's set manually as before, but
is part of uc list any way, with appropriate vid tag set. If vid_len
for real device is 0, the behaviour is the same as before this change,
so shouldn't be any impact on systems w/o individual virtual device
filtering (IVDF) enabled. This allows to control and sync vlan device
address and disable concrete vlan packet income when vlan interface is
down.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@xxxxxxxxxx>
---
[snip]
+static int vlan_dev_add_addr(struct net_device *dev, u8 *addr)
+{
+ struct net_device *real_dev = vlan_dev_real_dev(dev);
+ unsigned char naddr[ETH_ALEN + NET_8021Q_VID_TSIZE];
+
+ if (real_dev->vid_len) {
Don't you need to check that real_dev->vid_len is >= NET_8021Q_VID_TSIZE
here?
vid_len for all eth devices or 0 or NET_8021Q_VID_TSIZE and used here only as
a flag that different addressing scheme is used.
vlan_dev_set_addr_vid() do copy only < NET_8021Q_VID_TSIZE anyway.
Can add the following to be sure:
if (real_dev->vid_len) {
if (real_dev->vid_len != NET_8021Q_VID_TSIZE)
return -1;
....
}
But frankly, if this happens the system is ill and this check can't help it.
+ memcpy(naddr, addr, dev->addr_len);
+ vlan_dev_set_addr_vid(dev, naddr);
+ return dev_vid_uc_add(real_dev, naddr);
+ }
+
+ if (ether_addr_equal(addr, real_dev->dev_addr))
+ return 0;
+
+ return dev_uc_add(real_dev, addr);
+}
+
+static void vlan_dev_del_addr(struct net_device *dev, u8 *addr)
+{
+ struct net_device *real_dev = vlan_dev_real_dev(dev);
+ unsigned char naddr[ETH_ALEN + NET_8021Q_VID_TSIZE];
+
+ if (real_dev->vid_len) {
Same here.
Not same, it's void routine.
And del can't happen w/o add, no reason.
+ memcpy(naddr, addr, dev->addr_len);
+ vlan_dev_set_addr_vid(dev, naddr);
+ dev_vid_uc_del(real_dev, naddr);
+ return;
+ }
+
+ if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
+ dev_uc_del(real_dev, addr);
+}
+
+static int vlan_dev_subs_addr(struct net_device *dev, u8 *addr)
+{
+ int err;
+
+ err = vlan_dev_add_addr(dev, addr);
+ if (err < 0)
+ return err;
+
+ vlan_dev_del_addr(dev, dev->dev_addr);
+ return err;
+}
+
bool vlan_dev_inherit_address(struct net_device *dev,
struct net_device *real_dev)
{
if (dev->addr_assign_type != NET_ADDR_STOLEN)
return false;
+ if (real_dev->vid_len)
+ if (vlan_dev_subs_addr(dev, real_dev->dev_addr))
+ return false;
The check on real_dev->vid_len can be absorbed into vlan_dev_subs_addr()?
No, I'd tried. vlan_dev_subs_addr() is used not only here and move it under
makes more not combined code.
+
ether_addr_copy(dev->dev_addr, real_dev->dev_addr);
call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
return true;
@@ -278,9 +327,10 @@ static int vlan_dev_open(struct net_device *dev)
!(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
return -ENETDOWN;
- if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr) &&
- !vlan_dev_inherit_address(dev, real_dev)) {
- err = dev_uc_add(real_dev, dev->dev_addr);
+ if (ether_addr_equal(dev->dev_addr, real_dev->dev_addr) ||
+ (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr) &&
+ !vlan_dev_inherit_address(dev, real_dev))) {
Should this condition simply become if !vlan_dev_inherit_address() now?
It can't, I'd tried.
vlan_dev_inherit_address() is used in (vlan.c):
vlan_sync_address(struct net_device *dev, struct net_device *vlandev);
--
Regards,
Ivan Khoronzhuk
