Commit 8c4354ef authored by David S. Miller's avatar David S. Miller

Merge branch 'macvlan-Fix-some-issues-with-changing-mac-addresses'

Vladislav Yasevich says:

====================
macvlan: Fix some issues with changing mac addresses

There are some issues in macvlan wrt to changing it's mac address.
* An error is returned in the specified address is the same as an already
  assigned address.
* In passthru mode, the mac address of the macvlan device doesn't change.
* After changing the mac address of a passthru macvlan and then removing it,
  the mac address of the physical device remains changed.

This patch series attempts to resolve these issues.

V2: Address a small issue in p4 where we save the address from the lowerdev
    (from girish.moodalbail@oracle.com)
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents dfa523ae 18c8c54d
...@@ -39,16 +39,20 @@ ...@@ -39,16 +39,20 @@
#define MACVLAN_HASH_SIZE (1<<MACVLAN_HASH_BITS) #define MACVLAN_HASH_SIZE (1<<MACVLAN_HASH_BITS)
#define MACVLAN_BC_QUEUE_LEN 1000 #define MACVLAN_BC_QUEUE_LEN 1000
#define MACVLAN_F_PASSTHRU 1
#define MACVLAN_F_ADDRCHANGE 2
struct macvlan_port { struct macvlan_port {
struct net_device *dev; struct net_device *dev;
struct hlist_head vlan_hash[MACVLAN_HASH_SIZE]; struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
struct list_head vlans; struct list_head vlans;
struct sk_buff_head bc_queue; struct sk_buff_head bc_queue;
struct work_struct bc_work; struct work_struct bc_work;
bool passthru; u32 flags;
int count; int count;
struct hlist_head vlan_source_hash[MACVLAN_HASH_SIZE]; struct hlist_head vlan_source_hash[MACVLAN_HASH_SIZE];
DECLARE_BITMAP(mc_filter, MACVLAN_MC_FILTER_SZ); DECLARE_BITMAP(mc_filter, MACVLAN_MC_FILTER_SZ);
unsigned char perm_addr[ETH_ALEN];
}; };
struct macvlan_source_entry { struct macvlan_source_entry {
...@@ -66,6 +70,31 @@ struct macvlan_skb_cb { ...@@ -66,6 +70,31 @@ struct macvlan_skb_cb {
static void macvlan_port_destroy(struct net_device *dev); static void macvlan_port_destroy(struct net_device *dev);
static inline bool macvlan_passthru(const struct macvlan_port *port)
{
return port->flags & MACVLAN_F_PASSTHRU;
}
static inline void macvlan_set_passthru(struct macvlan_port *port)
{
port->flags |= MACVLAN_F_PASSTHRU;
}
static inline bool macvlan_addr_change(const struct macvlan_port *port)
{
return port->flags & MACVLAN_F_ADDRCHANGE;
}
static inline void macvlan_set_addr_change(struct macvlan_port *port)
{
port->flags |= MACVLAN_F_ADDRCHANGE;
}
static inline void macvlan_clear_addr_change(struct macvlan_port *port)
{
port->flags &= ~MACVLAN_F_ADDRCHANGE;
}
/* Hash Ethernet address */ /* Hash Ethernet address */
static u32 macvlan_eth_hash(const unsigned char *addr) static u32 macvlan_eth_hash(const unsigned char *addr)
{ {
...@@ -181,11 +210,12 @@ static void macvlan_hash_change_addr(struct macvlan_dev *vlan, ...@@ -181,11 +210,12 @@ static void macvlan_hash_change_addr(struct macvlan_dev *vlan,
static bool macvlan_addr_busy(const struct macvlan_port *port, static bool macvlan_addr_busy(const struct macvlan_port *port,
const unsigned char *addr) const unsigned char *addr)
{ {
/* Test to see if the specified multicast address is /* Test to see if the specified address is
* currently in use by the underlying device or * currently in use by the underlying device or
* another macvlan. * another macvlan.
*/ */
if (ether_addr_equal_64bits(port->dev->dev_addr, addr)) if (!macvlan_passthru(port) && !macvlan_addr_change(port) &&
ether_addr_equal_64bits(port->dev->dev_addr, addr))
return true; return true;
if (macvlan_hash_lookup(port, addr)) if (macvlan_hash_lookup(port, addr))
...@@ -445,7 +475,7 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb) ...@@ -445,7 +475,7 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
} }
macvlan_forward_source(skb, port, eth->h_source); macvlan_forward_source(skb, port, eth->h_source);
if (port->passthru) if (macvlan_passthru(port))
vlan = list_first_or_null_rcu(&port->vlans, vlan = list_first_or_null_rcu(&port->vlans,
struct macvlan_dev, list); struct macvlan_dev, list);
else else
...@@ -574,7 +604,7 @@ static int macvlan_open(struct net_device *dev) ...@@ -574,7 +604,7 @@ static int macvlan_open(struct net_device *dev)
struct net_device *lowerdev = vlan->lowerdev; struct net_device *lowerdev = vlan->lowerdev;
int err; int err;
if (vlan->port->passthru) { if (macvlan_passthru(vlan->port)) {
if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC)) { if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC)) {
err = dev_set_promiscuity(lowerdev, 1); err = dev_set_promiscuity(lowerdev, 1);
if (err < 0) if (err < 0)
...@@ -649,7 +679,7 @@ static int macvlan_stop(struct net_device *dev) ...@@ -649,7 +679,7 @@ static int macvlan_stop(struct net_device *dev)
dev_uc_unsync(lowerdev, dev); dev_uc_unsync(lowerdev, dev);
dev_mc_unsync(lowerdev, dev); dev_mc_unsync(lowerdev, dev);
if (vlan->port->passthru) { if (macvlan_passthru(vlan->port)) {
if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC)) if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC))
dev_set_promiscuity(lowerdev, -1); dev_set_promiscuity(lowerdev, -1);
goto hash_del; goto hash_del;
...@@ -672,6 +702,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) ...@@ -672,6 +702,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr)
{ {
struct macvlan_dev *vlan = netdev_priv(dev); struct macvlan_dev *vlan = netdev_priv(dev);
struct net_device *lowerdev = vlan->lowerdev; struct net_device *lowerdev = vlan->lowerdev;
struct macvlan_port *port = vlan->port;
int err; int err;
if (!(dev->flags & IFF_UP)) { if (!(dev->flags & IFF_UP)) {
...@@ -682,7 +713,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) ...@@ -682,7 +713,7 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr)
if (macvlan_addr_busy(vlan->port, addr)) if (macvlan_addr_busy(vlan->port, addr))
return -EBUSY; return -EBUSY;
if (!vlan->port->passthru) { if (!macvlan_passthru(port)) {
err = dev_uc_add(lowerdev, addr); err = dev_uc_add(lowerdev, addr);
if (err) if (err)
return err; return err;
...@@ -692,6 +723,15 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr) ...@@ -692,6 +723,15 @@ static int macvlan_sync_address(struct net_device *dev, unsigned char *addr)
macvlan_hash_change_addr(vlan, addr); macvlan_hash_change_addr(vlan, addr);
} }
if (macvlan_passthru(port) && !macvlan_addr_change(port)) {
/* Since addr_change isn't set, we are here due to lower
* device change. Save the lower-dev address so we can
* restore it later.
*/
ether_addr_copy(vlan->port->perm_addr,
lowerdev->dev_addr);
}
macvlan_clear_addr_change(port);
return 0; return 0;
} }
...@@ -703,7 +743,12 @@ static int macvlan_set_mac_address(struct net_device *dev, void *p) ...@@ -703,7 +743,12 @@ static int macvlan_set_mac_address(struct net_device *dev, void *p)
if (!is_valid_ether_addr(addr->sa_data)) if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL; return -EADDRNOTAVAIL;
/* If the addresses are the same, this is a no-op */
if (ether_addr_equal(dev->dev_addr, addr->sa_data))
return 0;
if (vlan->mode == MACVLAN_MODE_PASSTHRU) { if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
macvlan_set_addr_change(vlan->port);
dev_set_mac_address(vlan->lowerdev, addr); dev_set_mac_address(vlan->lowerdev, addr);
return 0; return 0;
} }
...@@ -928,7 +973,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], ...@@ -928,7 +973,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
/* Support unicast filter only on passthru devices. /* Support unicast filter only on passthru devices.
* Multicast filter should be allowed on all devices. * Multicast filter should be allowed on all devices.
*/ */
if (!vlan->port->passthru && is_unicast_ether_addr(addr)) if (!macvlan_passthru(vlan->port) && is_unicast_ether_addr(addr))
return -EOPNOTSUPP; return -EOPNOTSUPP;
if (flags & NLM_F_REPLACE) if (flags & NLM_F_REPLACE)
...@@ -952,7 +997,7 @@ static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[], ...@@ -952,7 +997,7 @@ static int macvlan_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
/* Support unicast filter only on passthru devices. /* Support unicast filter only on passthru devices.
* Multicast filter should be allowed on all devices. * Multicast filter should be allowed on all devices.
*/ */
if (!vlan->port->passthru && is_unicast_ether_addr(addr)) if (!macvlan_passthru(vlan->port) && is_unicast_ether_addr(addr))
return -EOPNOTSUPP; return -EOPNOTSUPP;
if (is_unicast_ether_addr(addr)) if (is_unicast_ether_addr(addr))
...@@ -1120,8 +1165,8 @@ static int macvlan_port_create(struct net_device *dev) ...@@ -1120,8 +1165,8 @@ static int macvlan_port_create(struct net_device *dev)
if (port == NULL) if (port == NULL)
return -ENOMEM; return -ENOMEM;
port->passthru = false;
port->dev = dev; port->dev = dev;
ether_addr_copy(port->perm_addr, dev->dev_addr);
INIT_LIST_HEAD(&port->vlans); INIT_LIST_HEAD(&port->vlans);
for (i = 0; i < MACVLAN_HASH_SIZE; i++) for (i = 0; i < MACVLAN_HASH_SIZE; i++)
INIT_HLIST_HEAD(&port->vlan_hash[i]); INIT_HLIST_HEAD(&port->vlan_hash[i]);
...@@ -1161,6 +1206,18 @@ static void macvlan_port_destroy(struct net_device *dev) ...@@ -1161,6 +1206,18 @@ static void macvlan_port_destroy(struct net_device *dev)
kfree_skb(skb); kfree_skb(skb);
} }
/* If the lower device address has been changed by passthru
* macvlan, put it back.
*/
if (macvlan_passthru(port) &&
!ether_addr_equal(port->dev->dev_addr, port->perm_addr)) {
struct sockaddr sa;
sa.sa_family = port->dev->type;
memcpy(&sa.sa_data, port->perm_addr, port->dev->addr_len);
dev_set_mac_address(port->dev, &sa);
}
kfree(port); kfree(port);
} }
...@@ -1326,7 +1383,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, ...@@ -1326,7 +1383,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
port = macvlan_port_get_rtnl(lowerdev); port = macvlan_port_get_rtnl(lowerdev);
/* Only 1 macvlan device can be created in passthru mode */ /* Only 1 macvlan device can be created in passthru mode */
if (port->passthru) { if (macvlan_passthru(port)) {
/* The macvlan port must be not created this time, /* The macvlan port must be not created this time,
* still goto destroy_macvlan_port for readability. * still goto destroy_macvlan_port for readability.
*/ */
...@@ -1352,7 +1409,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev, ...@@ -1352,7 +1409,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
err = -EINVAL; err = -EINVAL;
goto destroy_macvlan_port; goto destroy_macvlan_port;
} }
port->passthru = true; macvlan_set_passthru(port);
eth_hw_addr_inherit(dev, lowerdev); eth_hw_addr_inherit(dev, lowerdev);
} }
...@@ -1434,7 +1491,7 @@ static int macvlan_changelink(struct net_device *dev, ...@@ -1434,7 +1491,7 @@ static int macvlan_changelink(struct net_device *dev,
if (data && data[IFLA_MACVLAN_FLAGS]) { if (data && data[IFLA_MACVLAN_FLAGS]) {
__u16 flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]); __u16 flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
bool promisc = (flags ^ vlan->flags) & MACVLAN_FLAG_NOPROMISC; bool promisc = (flags ^ vlan->flags) & MACVLAN_FLAG_NOPROMISC;
if (vlan->port->passthru && promisc) { if (macvlan_passthru(vlan->port) && promisc) {
int err; int err;
if (flags & MACVLAN_FLAG_NOPROMISC) if (flags & MACVLAN_FLAG_NOPROMISC)
...@@ -1597,7 +1654,7 @@ static int macvlan_device_event(struct notifier_block *unused, ...@@ -1597,7 +1654,7 @@ static int macvlan_device_event(struct notifier_block *unused,
} }
break; break;
case NETDEV_CHANGEADDR: case NETDEV_CHANGEADDR:
if (!port->passthru) if (!macvlan_passthru(port))
return NOTIFY_DONE; return NOTIFY_DONE;
vlan = list_first_entry_or_null(&port->vlans, vlan = list_first_entry_or_null(&port->vlans,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment