* [ETH 01/04]: Validate address in eth_mac_addr
2007-07-11 17:38 [RTNETLINK 00/04]: rtnl_link improvements Patrick McHardy
@ 2007-07-11 17:38 ` Patrick McHardy
2007-07-12 2:41 ` David Miller
2007-07-11 17:38 ` [VLAN 02/04]: Fix MAC address handling Patrick McHardy
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2007-07-11 17:38 UTC (permalink / raw)
To: davem; +Cc: netdev, greearb, Patrick McHardy, xemul
[ETH]: Validate address in eth_mac_addr
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 690fb4dbafa9f60f8cb520d5def544580107b3a4
tree ded62c0fcd8497b5cf9459213e1579cc700abb39
parent 99d24edeb6abc6ca3a0d0fbdb83c664c04403c8c
author Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:23:22 +0200
committer Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:23:22 +0200
net/ethernet/eth.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 1387e54..12c7657 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -266,8 +266,11 @@ void eth_header_cache_update(struct hh_cache *hh, struct net_device *dev,
static int eth_mac_addr(struct net_device *dev, void *p)
{
struct sockaddr *addr = p;
+
if (netif_running(dev))
return -EBUSY;
+ if (!is_valid_ether_addr(addr->sa_data))
+ return -EADDRNOTAVAIL;
memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
return 0;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* [VLAN 02/04]: Fix MAC address handling
2007-07-11 17:38 [RTNETLINK 00/04]: rtnl_link improvements Patrick McHardy
2007-07-11 17:38 ` [ETH 01/04]: Validate address in eth_mac_addr Patrick McHardy
@ 2007-07-11 17:38 ` Patrick McHardy
2007-07-11 17:54 ` Patrick McHardy
2007-07-12 2:41 ` David Miller
2007-07-11 17:38 ` [RTNETLINK 03/04]: rtnl_link API simplification Patrick McHardy
2007-07-11 17:38 ` [RTNETLINK 04/04]: rtnl_link: allow specifying initial device address Patrick McHardy
3 siblings, 2 replies; 13+ messages in thread
From: Patrick McHardy @ 2007-07-11 17:38 UTC (permalink / raw)
To: davem; +Cc: netdev, greearb, Patrick McHardy, xemul
[VLAN]: Fix MAC address handling
The VLAN MAC address handling is broken in multiple ways. When the address
differs when setting it, the real device is put in promiscous mode twice,
but never taken out again. Additionally it doesn't resync when the real
device's address is changed and needlessly puts it in promiscous mode when
the vlan device is still down.
Fix by moving address handling to vlan_dev_open/vlan_dev_stop and properly
deal with address changes in the device notifier. Also switch to
dev_unicast_add (which needs the exact same handling).
Since the set_mac_address handler is identical to the generic ethernet one
with these changes, kill it and use ether_setup().
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit f5402e5ada051f17384c089fc49b247fa62e9723
tree 9e4642678929a19229893c7c8656cf11e1ad7401
parent 690fb4dbafa9f60f8cb520d5def544580107b3a4
author Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:27:21 +0200
committer Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:27:21 +0200
include/linux/if_vlan.h | 1 +
net/8021q/vlan.c | 35 +++++++++++++++++++++++++----
net/8021q/vlan.h | 1 -
net/8021q/vlan_dev.c | 57 +++++++++++++++--------------------------------
4 files changed, 49 insertions(+), 45 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index c791287..61a57dc 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -135,6 +135,7 @@ struct vlan_dev_info {
int old_allmulti; /* similar to above. */
int old_promiscuity; /* similar to above. */
struct net_device *real_dev; /* the underlying device/interface */
+ unsigned char real_dev_addr[ETH_ALEN];
struct proc_dir_entry *dent; /* Holds the proc data */
unsigned long cnt_inc_headroom_on_tx; /* How many times did we have to grow the skb on TX. */
unsigned long cnt_encap_on_xmit; /* How many times did we have to encapsulate the skb on TX. */
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index e7583ee..c5e5655 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -345,12 +345,8 @@ static int vlan_dev_init(struct net_device *dev)
(1<<__LINK_STATE_DORMANT))) |
(1<<__LINK_STATE_PRESENT);
- /* TODO: maybe just assign it to be ETHERNET? */
- dev->type = real_dev->type;
-
memcpy(dev->broadcast, real_dev->broadcast, real_dev->addr_len);
memcpy(dev->dev_addr, real_dev->dev_addr, real_dev->addr_len);
- dev->addr_len = real_dev->addr_len;
if (real_dev->features & NETIF_F_HW_VLAN_TX) {
dev->hard_header = real_dev->hard_header;
@@ -364,6 +360,7 @@ static int vlan_dev_init(struct net_device *dev)
dev->rebuild_header = vlan_dev_rebuild_header;
}
dev->hard_header_parse = real_dev->hard_header_parse;
+ dev->hard_header_cache = NULL;
lockdep_set_class(&dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
return 0;
@@ -373,6 +370,8 @@ void vlan_setup(struct net_device *new_dev)
{
SET_MODULE_OWNER(new_dev);
+ ether_setup(new_dev);
+
/* new_dev->ifindex = 0; it will be set when added to
* the global list.
* iflink is set as well.
@@ -392,7 +391,6 @@ void vlan_setup(struct net_device *new_dev)
new_dev->init = vlan_dev_init;
new_dev->open = vlan_dev_open;
new_dev->stop = vlan_dev_stop;
- new_dev->set_mac_address = vlan_dev_set_mac_address;
new_dev->set_multicast_list = vlan_dev_set_multicast_list;
new_dev->destructor = free_netdev;
new_dev->do_ioctl = vlan_dev_ioctl;
@@ -592,6 +590,22 @@ out_free_newdev:
return err;
}
+static void vlan_sync_address(struct net_device *dev,
+ struct net_device *vlandev)
+{
+ struct vlan_dev_info *vlan = VLAN_DEV_INFO(vlandev);
+
+ if (!compare_ether_addr(vlan->real_dev_addr, dev->dev_addr))
+ return;
+
+ if (compare_ether_addr(vlandev->dev_addr, dev->dev_addr))
+ dev_unicast_add(dev, vlandev->dev_addr, ETH_ALEN);
+ else
+ dev_unicast_delete(dev, vlandev->dev_addr, ETH_ALEN);
+
+ memcpy(vlan->real_dev_addr, dev->dev_addr, ETH_ALEN);
+}
+
static int vlan_device_event(struct notifier_block *unused, unsigned long event, void *ptr)
{
struct net_device *dev = ptr;
@@ -618,6 +632,17 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
}
break;
+ case NETDEV_CHANGEADDR:
+ /* Adjust unicast filters on underlying device */
+ for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
+ vlandev = vlan_group_get_device(grp, i);
+ if (!vlandev)
+ continue;
+
+ vlan_sync_address(dev, vlandev);
+ }
+ break;
+
case NETDEV_DOWN:
/* Put all VLANs for this dev in the down state too. */
for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index fe6bb0f..62ce1c5 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -58,7 +58,6 @@ int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
int vlan_dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev);
int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb, struct net_device *dev);
int vlan_dev_change_mtu(struct net_device *dev, int new_mtu);
-int vlan_dev_set_mac_address(struct net_device *dev, void* addr);
int vlan_dev_open(struct net_device* dev);
int vlan_dev_stop(struct net_device* dev);
int vlan_dev_ioctl(struct net_device* dev, struct ifreq *ifr, int cmd);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 95afe38..d4a62d1 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -612,44 +612,6 @@ void vlan_dev_get_vid(const struct net_device *dev, unsigned short *result)
*result = VLAN_DEV_INFO(dev)->vlan_id;
}
-int vlan_dev_set_mac_address(struct net_device *dev, void *addr_struct_p)
-{
- struct sockaddr *addr = (struct sockaddr *)(addr_struct_p);
- int i;
-
- if (netif_running(dev))
- return -EBUSY;
-
- memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
-
- printk("%s: Setting MAC address to ", dev->name);
- for (i = 0; i < 6; i++)
- printk(" %2.2x", dev->dev_addr[i]);
- printk(".\n");
-
- if (memcmp(VLAN_DEV_INFO(dev)->real_dev->dev_addr,
- dev->dev_addr,
- dev->addr_len) != 0) {
- if (!(VLAN_DEV_INFO(dev)->real_dev->flags & IFF_PROMISC)) {
- int flgs = VLAN_DEV_INFO(dev)->real_dev->flags;
-
- /* Increment our in-use promiscuity counter */
- dev_set_promiscuity(VLAN_DEV_INFO(dev)->real_dev, 1);
-
- /* Make PROMISC visible to the user. */
- flgs |= IFF_PROMISC;
- printk("VLAN (%s): Setting underlying device (%s) to promiscious mode.\n",
- dev->name, VLAN_DEV_INFO(dev)->real_dev->name);
- dev_change_flags(VLAN_DEV_INFO(dev)->real_dev, flgs);
- }
- } else {
- printk("VLAN (%s): Underlying device (%s) has same MAC, not checking promiscious mode.\n",
- dev->name, VLAN_DEV_INFO(dev)->real_dev->name);
- }
-
- return 0;
-}
-
static inline int vlan_dmi_equals(struct dev_mc_list *dmi1,
struct dev_mc_list *dmi2)
{
@@ -736,15 +698,32 @@ static void vlan_flush_mc_list(struct net_device *dev)
int vlan_dev_open(struct net_device *dev)
{
- if (!(VLAN_DEV_INFO(dev)->real_dev->flags & IFF_UP))
+ struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
+ struct net_device *real_dev = vlan->real_dev;
+ int err;
+
+ if (!(real_dev->flags & IFF_UP))
return -ENETDOWN;
+ if (compare_ether_addr(dev->dev_addr, real_dev->dev_addr)) {
+ err = dev_unicast_add(real_dev, dev->dev_addr, ETH_ALEN);
+ if (err < 0)
+ return err;
+ }
+ memcpy(vlan->real_dev_addr, real_dev->dev_addr, ETH_ALEN);
+
return 0;
}
int vlan_dev_stop(struct net_device *dev)
{
+ struct net_device *real_dev = VLAN_DEV_INFO(dev)->real_dev;
+
vlan_flush_mc_list(dev);
+
+ if (compare_ether_addr(dev->dev_addr, real_dev->dev_addr))
+ dev_unicast_delete(real_dev, dev->dev_addr, dev->addr_len);
+
return 0;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [VLAN 02/04]: Fix MAC address handling
2007-07-11 17:38 ` [VLAN 02/04]: Fix MAC address handling Patrick McHardy
@ 2007-07-11 17:54 ` Patrick McHardy
2007-07-11 18:16 ` Ben Greear
2007-07-12 2:45 ` David Miller
2007-07-12 2:41 ` David Miller
1 sibling, 2 replies; 13+ messages in thread
From: Patrick McHardy @ 2007-07-11 17:54 UTC (permalink / raw)
To: davem; +Cc: netdev, greearb, xemul
[-- Attachment #1: Type: text/plain, Size: 816 bytes --]
Patrick McHardy wrote:
> [VLAN]: Fix MAC address handling
>
> +static void vlan_sync_address(struct net_device *dev,
> + struct net_device *vlandev)
> +{
> + struct vlan_dev_info *vlan = VLAN_DEV_INFO(vlandev);
> +
> + if (!compare_ether_addr(vlan->real_dev_addr, dev->dev_addr))
> + return;
> +
> + if (compare_ether_addr(vlandev->dev_addr, dev->dev_addr))
> + dev_unicast_add(dev, vlandev->dev_addr, ETH_ALEN);
> + else
> + dev_unicast_delete(dev, vlandev->dev_addr, ETH_ALEN);
> +
> + memcpy(vlan->real_dev_addr, dev->dev_addr, ETH_ALEN);
> +}
Unfortunately the one case I didn't test is still wrong :|
The above synchronization incorrectly removes the address in case
if was different before and is still different afterwards. This
patch is fixed and contains a few comments as an added bonus :)
[-- Attachment #2: 02.diff --]
[-- Type: text/x-diff, Size: 8172 bytes --]
[VLAN]: Fix MAC address handling
The VLAN MAC address handling is broken in multiple ways. When the address
differs when setting it, the real device is put in promiscous mode twice,
but never taken out again. Additionally it doesn't resync when the real
device's address is changed and needlessly puts it in promiscous mode when
the vlan device is still down.
Fix by moving address handling to vlan_dev_open/vlan_dev_stop and properly
deal with address changes in the device notifier. Also switch to
dev_unicast_add (which needs the exact same handling).
Since the set_mac_address handler is identical to the generic ethernet one
with these changes, kill it and use ether_setup().
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 7d0a5d2f8d28ebe95cda8d2e16bf6d8030efc8b3
tree d91afa76b1aaaaa069a435967b8627f6aa820e13
parent 690fb4dbafa9f60f8cb520d5def544580107b3a4
author Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:52:08 +0200
committer Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:52:08 +0200
include/linux/if_vlan.h | 1 +
net/8021q/vlan.c | 43 +++++++++++++++++++++++++++++++----
net/8021q/vlan.h | 1 -
net/8021q/vlan_dev.c | 57 +++++++++++++++--------------------------------
4 files changed, 57 insertions(+), 45 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index c791287..61a57dc 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -135,6 +135,7 @@ struct vlan_dev_info {
int old_allmulti; /* similar to above. */
int old_promiscuity; /* similar to above. */
struct net_device *real_dev; /* the underlying device/interface */
+ unsigned char real_dev_addr[ETH_ALEN];
struct proc_dir_entry *dent; /* Holds the proc data */
unsigned long cnt_inc_headroom_on_tx; /* How many times did we have to grow the skb on TX. */
unsigned long cnt_encap_on_xmit; /* How many times did we have to encapsulate the skb on TX. */
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index e7583ee..b463ba4 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -345,12 +345,8 @@ static int vlan_dev_init(struct net_device *dev)
(1<<__LINK_STATE_DORMANT))) |
(1<<__LINK_STATE_PRESENT);
- /* TODO: maybe just assign it to be ETHERNET? */
- dev->type = real_dev->type;
-
memcpy(dev->broadcast, real_dev->broadcast, real_dev->addr_len);
memcpy(dev->dev_addr, real_dev->dev_addr, real_dev->addr_len);
- dev->addr_len = real_dev->addr_len;
if (real_dev->features & NETIF_F_HW_VLAN_TX) {
dev->hard_header = real_dev->hard_header;
@@ -364,6 +360,7 @@ static int vlan_dev_init(struct net_device *dev)
dev->rebuild_header = vlan_dev_rebuild_header;
}
dev->hard_header_parse = real_dev->hard_header_parse;
+ dev->hard_header_cache = NULL;
lockdep_set_class(&dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
return 0;
@@ -373,6 +370,8 @@ void vlan_setup(struct net_device *new_dev)
{
SET_MODULE_OWNER(new_dev);
+ ether_setup(new_dev);
+
/* new_dev->ifindex = 0; it will be set when added to
* the global list.
* iflink is set as well.
@@ -392,7 +391,6 @@ void vlan_setup(struct net_device *new_dev)
new_dev->init = vlan_dev_init;
new_dev->open = vlan_dev_open;
new_dev->stop = vlan_dev_stop;
- new_dev->set_mac_address = vlan_dev_set_mac_address;
new_dev->set_multicast_list = vlan_dev_set_multicast_list;
new_dev->destructor = free_netdev;
new_dev->do_ioctl = vlan_dev_ioctl;
@@ -592,6 +590,30 @@ out_free_newdev:
return err;
}
+static void vlan_sync_address(struct net_device *dev,
+ struct net_device *vlandev)
+{
+ struct vlan_dev_info *vlan = VLAN_DEV_INFO(vlandev);
+
+ /* May be called without an actual change */
+ if (!compare_ether_addr(vlan->real_dev_addr, dev->dev_addr))
+ return;
+
+ /* vlan address was different from the old address and is equal to
+ * the new address */
+ if (compare_ether_addr(vlandev->dev_addr, vlan->real_dev_addr) &&
+ !compare_ether_addr(vlandev->dev_addr, dev->dev_addr))
+ dev_unicast_delete(dev, vlandev->dev_addr, ETH_ALEN);
+
+ /* vlan address was equal to the old address and is different from
+ * the new address */
+ if (!compare_ether_addr(vlandev->dev_addr, vlan->real_dev_addr) &&
+ compare_ether_addr(vlandev->dev_addr, dev->dev_addr))
+ dev_unicast_add(dev, vlandev->dev_addr, ETH_ALEN);
+
+ memcpy(vlan->real_dev_addr, dev->dev_addr, ETH_ALEN);
+}
+
static int vlan_device_event(struct notifier_block *unused, unsigned long event, void *ptr)
{
struct net_device *dev = ptr;
@@ -618,6 +640,17 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
}
break;
+ case NETDEV_CHANGEADDR:
+ /* Adjust unicast filters on underlying device */
+ for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
+ vlandev = vlan_group_get_device(grp, i);
+ if (!vlandev)
+ continue;
+
+ vlan_sync_address(dev, vlandev);
+ }
+ break;
+
case NETDEV_DOWN:
/* Put all VLANs for this dev in the down state too. */
for (i = 0; i < VLAN_GROUP_ARRAY_LEN; i++) {
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index fe6bb0f..62ce1c5 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -58,7 +58,6 @@ int vlan_dev_hard_header(struct sk_buff *skb, struct net_device *dev,
int vlan_dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev);
int vlan_dev_hwaccel_hard_start_xmit(struct sk_buff *skb, struct net_device *dev);
int vlan_dev_change_mtu(struct net_device *dev, int new_mtu);
-int vlan_dev_set_mac_address(struct net_device *dev, void* addr);
int vlan_dev_open(struct net_device* dev);
int vlan_dev_stop(struct net_device* dev);
int vlan_dev_ioctl(struct net_device* dev, struct ifreq *ifr, int cmd);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 95afe38..d4a62d1 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -612,44 +612,6 @@ void vlan_dev_get_vid(const struct net_device *dev, unsigned short *result)
*result = VLAN_DEV_INFO(dev)->vlan_id;
}
-int vlan_dev_set_mac_address(struct net_device *dev, void *addr_struct_p)
-{
- struct sockaddr *addr = (struct sockaddr *)(addr_struct_p);
- int i;
-
- if (netif_running(dev))
- return -EBUSY;
-
- memcpy(dev->dev_addr, addr->sa_data, dev->addr_len);
-
- printk("%s: Setting MAC address to ", dev->name);
- for (i = 0; i < 6; i++)
- printk(" %2.2x", dev->dev_addr[i]);
- printk(".\n");
-
- if (memcmp(VLAN_DEV_INFO(dev)->real_dev->dev_addr,
- dev->dev_addr,
- dev->addr_len) != 0) {
- if (!(VLAN_DEV_INFO(dev)->real_dev->flags & IFF_PROMISC)) {
- int flgs = VLAN_DEV_INFO(dev)->real_dev->flags;
-
- /* Increment our in-use promiscuity counter */
- dev_set_promiscuity(VLAN_DEV_INFO(dev)->real_dev, 1);
-
- /* Make PROMISC visible to the user. */
- flgs |= IFF_PROMISC;
- printk("VLAN (%s): Setting underlying device (%s) to promiscious mode.\n",
- dev->name, VLAN_DEV_INFO(dev)->real_dev->name);
- dev_change_flags(VLAN_DEV_INFO(dev)->real_dev, flgs);
- }
- } else {
- printk("VLAN (%s): Underlying device (%s) has same MAC, not checking promiscious mode.\n",
- dev->name, VLAN_DEV_INFO(dev)->real_dev->name);
- }
-
- return 0;
-}
-
static inline int vlan_dmi_equals(struct dev_mc_list *dmi1,
struct dev_mc_list *dmi2)
{
@@ -736,15 +698,32 @@ static void vlan_flush_mc_list(struct net_device *dev)
int vlan_dev_open(struct net_device *dev)
{
- if (!(VLAN_DEV_INFO(dev)->real_dev->flags & IFF_UP))
+ struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
+ struct net_device *real_dev = vlan->real_dev;
+ int err;
+
+ if (!(real_dev->flags & IFF_UP))
return -ENETDOWN;
+ if (compare_ether_addr(dev->dev_addr, real_dev->dev_addr)) {
+ err = dev_unicast_add(real_dev, dev->dev_addr, ETH_ALEN);
+ if (err < 0)
+ return err;
+ }
+ memcpy(vlan->real_dev_addr, real_dev->dev_addr, ETH_ALEN);
+
return 0;
}
int vlan_dev_stop(struct net_device *dev)
{
+ struct net_device *real_dev = VLAN_DEV_INFO(dev)->real_dev;
+
vlan_flush_mc_list(dev);
+
+ if (compare_ether_addr(dev->dev_addr, real_dev->dev_addr))
+ dev_unicast_delete(real_dev, dev->dev_addr, dev->addr_len);
+
return 0;
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [VLAN 02/04]: Fix MAC address handling
2007-07-11 17:54 ` Patrick McHardy
@ 2007-07-11 18:16 ` Ben Greear
2007-07-11 18:37 ` Patrick McHardy
2007-07-12 2:45 ` David Miller
1 sibling, 1 reply; 13+ messages in thread
From: Ben Greear @ 2007-07-11 18:16 UTC (permalink / raw)
To: Patrick McHardy; +Cc: davem, netdev, xemul
Patrick McHardy wrote:
> Patrick McHardy wrote:
>> [VLAN]: Fix MAC address handling
>>
>> +static void vlan_sync_address(struct net_device *dev,
>> + struct net_device *vlandev)
>> +{
>> + struct vlan_dev_info *vlan = VLAN_DEV_INFO(vlandev);
>> +
>> + if (!compare_ether_addr(vlan->real_dev_addr, dev->dev_addr))
>> + return;
>> +
>> + if (compare_ether_addr(vlandev->dev_addr, dev->dev_addr))
>> + dev_unicast_add(dev, vlandev->dev_addr, ETH_ALEN);
>> + else
>> + dev_unicast_delete(dev, vlandev->dev_addr, ETH_ALEN);
>> +
>> + memcpy(vlan->real_dev_addr, dev->dev_addr, ETH_ALEN);
>> +}
>
>
> Unfortunately the one case I didn't test is still wrong :|
>
> The above synchronization incorrectly removes the address in case
> if was different before and is still different afterwards. This
> patch is fixed and contains a few comments as an added bonus :)
The new patch looks good to me..though this is some tricky code
so I might have missed something...
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [VLAN 02/04]: Fix MAC address handling
2007-07-11 17:54 ` Patrick McHardy
2007-07-11 18:16 ` Ben Greear
@ 2007-07-12 2:45 ` David Miller
1 sibling, 0 replies; 13+ messages in thread
From: David Miller @ 2007-07-12 2:45 UTC (permalink / raw)
To: kaber; +Cc: netdev, greearb, xemul
From: Patrick McHardy <kaber@trash.net>
Date: Wed, 11 Jul 2007 19:54:18 +0200
> Patrick McHardy wrote:
> > [VLAN]: Fix MAC address handling
> >
> > +static void vlan_sync_address(struct net_device *dev,
> > + struct net_device *vlandev)
> > +{
> > + struct vlan_dev_info *vlan = VLAN_DEV_INFO(vlandev);
> > +
> > + if (!compare_ether_addr(vlan->real_dev_addr, dev->dev_addr))
> > + return;
> > +
> > + if (compare_ether_addr(vlandev->dev_addr, dev->dev_addr))
> > + dev_unicast_add(dev, vlandev->dev_addr, ETH_ALEN);
> > + else
> > + dev_unicast_delete(dev, vlandev->dev_addr, ETH_ALEN);
> > +
> > + memcpy(vlan->real_dev_addr, dev->dev_addr, ETH_ALEN);
> > +}
>
>
> Unfortunately the one case I didn't test is still wrong :|
>
> The above synchronization incorrectly removes the address in case
> if was different before and is still different afterwards. This
> patch is fixed and contains a few comments as an added bonus :)
So there is no confusion, I made sure to use this updated
version of the patch. :)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [VLAN 02/04]: Fix MAC address handling
2007-07-11 17:38 ` [VLAN 02/04]: Fix MAC address handling Patrick McHardy
2007-07-11 17:54 ` Patrick McHardy
@ 2007-07-12 2:41 ` David Miller
1 sibling, 0 replies; 13+ messages in thread
From: David Miller @ 2007-07-12 2:41 UTC (permalink / raw)
To: kaber; +Cc: netdev, greearb, xemul
From: Patrick McHardy <kaber@trash.net>
Date: Wed, 11 Jul 2007 19:38:16 +0200 (MEST)
> [VLAN]: Fix MAC address handling
>
> The VLAN MAC address handling is broken in multiple ways. When the address
> differs when setting it, the real device is put in promiscous mode twice,
> but never taken out again. Additionally it doesn't resync when the real
> device's address is changed and needlessly puts it in promiscous mode when
> the vlan device is still down.
>
> Fix by moving address handling to vlan_dev_open/vlan_dev_stop and properly
> deal with address changes in the device notifier. Also switch to
> dev_unicast_add (which needs the exact same handling).
>
> Since the set_mac_address handler is identical to the generic ethernet one
> with these changes, kill it and use ether_setup().
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
Applied, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RTNETLINK 03/04]: rtnl_link API simplification
2007-07-11 17:38 [RTNETLINK 00/04]: rtnl_link improvements Patrick McHardy
2007-07-11 17:38 ` [ETH 01/04]: Validate address in eth_mac_addr Patrick McHardy
2007-07-11 17:38 ` [VLAN 02/04]: Fix MAC address handling Patrick McHardy
@ 2007-07-11 17:38 ` Patrick McHardy
2007-07-12 2:42 ` David Miller
2007-07-11 17:38 ` [RTNETLINK 04/04]: rtnl_link: allow specifying initial device address Patrick McHardy
3 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2007-07-11 17:38 UTC (permalink / raw)
To: davem; +Cc: netdev, greearb, Patrick McHardy, xemul
[RTNETLINK]: rtnl_link API simplification
All drivers need to unregister their devices in the module unload function.
While doing so they must hold the rtnl and atomically unregister the
rtnl_link ops as well. This makes the rtnl_link_unregister function that
takes the rtnl itself completely useless.
Provide default newlink/dellink functions, make __rtnl_link_unregister and
rtnl_link_unregister unregister all devices with matching rtnl_link_ops and
change the existing users to take advantage of that.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 88fcde1e75dbfad7e7210642c0364850eed2591e
tree 0d1fd7e324bc69b02a01d8499a48abb6aeae5704
parent f5402e5ada051f17384c089fc49b247fa62e9723
author Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:27:32 +0200
committer Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:27:32 +0200
drivers/net/dummy.c | 57 +++----------------------------------------------
drivers/net/ifb.c | 58 +++++---------------------------------------------
net/8021q/vlan.c | 21 ------------------
net/core/rtnetlink.c | 18 ++++++++++++----
4 files changed, 23 insertions(+), 131 deletions(-)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 91126b9..373ff70 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -37,11 +37,6 @@
#include <linux/rtnetlink.h>
#include <net/rtnetlink.h>
-struct dummy_priv {
- struct net_device *dev;
- struct list_head list;
-};
-
static int numdummies = 1;
static int dummy_xmit(struct sk_buff *skb, struct net_device *dev);
@@ -89,37 +84,9 @@ static int dummy_xmit(struct sk_buff *skb, struct net_device *dev)
return 0;
}
-static LIST_HEAD(dummies);
-
-static int dummy_newlink(struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[])
-{
- struct dummy_priv *priv = netdev_priv(dev);
- int err;
-
- err = register_netdevice(dev);
- if (err < 0)
- return err;
-
- priv->dev = dev;
- list_add_tail(&priv->list, &dummies);
- return 0;
-}
-
-static void dummy_dellink(struct net_device *dev)
-{
- struct dummy_priv *priv = netdev_priv(dev);
-
- list_del(&priv->list);
- unregister_netdevice(dev);
-}
-
static struct rtnl_link_ops dummy_link_ops __read_mostly = {
.kind = "dummy",
- .priv_size = sizeof(struct dummy_priv),
.setup = dummy_setup,
- .newlink = dummy_newlink,
- .dellink = dummy_dellink,
};
/* Number of dummy devices to be set up by this module. */
@@ -129,12 +96,9 @@ MODULE_PARM_DESC(numdummies, "Number of dummy pseudo devices");
static int __init dummy_init_one(void)
{
struct net_device *dev_dummy;
- struct dummy_priv *priv;
int err;
- dev_dummy = alloc_netdev(sizeof(struct dummy_priv), "dummy%d",
- dummy_setup);
-
+ dev_dummy = alloc_netdev(0, "dummy%d", dummy_setup);
if (!dev_dummy)
return -ENOMEM;
@@ -146,10 +110,6 @@ static int __init dummy_init_one(void)
err = register_netdevice(dev_dummy);
if (err < 0)
goto err;
-
- priv = netdev_priv(dev_dummy);
- priv->dev = dev_dummy;
- list_add_tail(&priv->list, &dummies);
return 0;
err:
@@ -159,7 +119,6 @@ err:
static int __init dummy_init_module(void)
{
- struct dummy_priv *priv, *next;
int i, err = 0;
rtnl_lock();
@@ -167,11 +126,8 @@ static int __init dummy_init_module(void)
for (i = 0; i < numdummies && !err; i++)
err = dummy_init_one();
- if (err < 0) {
- list_for_each_entry_safe(priv, next, &dummies, list)
- dummy_dellink(priv->dev);
+ if (err < 0)
__rtnl_link_unregister(&dummy_link_ops);
- }
rtnl_unlock();
return err;
@@ -179,14 +135,7 @@ static int __init dummy_init_module(void)
static void __exit dummy_cleanup_module(void)
{
- struct dummy_priv *priv, *next;
-
- rtnl_lock();
- list_for_each_entry_safe(priv, next, &dummies, list)
- dummy_dellink(priv->dev);
-
- __rtnl_link_unregister(&dummy_link_ops);
- rtnl_unlock();
+ rtnl_link_unregister(&dummy_link_ops);
}
module_init(dummy_init_module);
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 669ee1a..c8e7c8f 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -33,15 +33,12 @@
#include <linux/etherdevice.h>
#include <linux/init.h>
#include <linux/moduleparam.h>
-#include <linux/list.h>
#include <net/pkt_sched.h>
#define TX_TIMEOUT (2*HZ)
#define TX_Q_LIMIT 32
struct ifb_private {
- struct list_head list;
- struct net_device *dev;
struct net_device_stats stats;
struct tasklet_struct ifb_tasklet;
int tasklet_pending;
@@ -201,12 +198,6 @@ static struct net_device_stats *ifb_get_stats(struct net_device *dev)
return stats;
}
-static LIST_HEAD(ifbs);
-
-/* Number of ifb devices to be set up by this module. */
-module_param(numifbs, int, 0);
-MODULE_PARM_DESC(numifbs, "Number of ifb devices");
-
static int ifb_close(struct net_device *dev)
{
struct ifb_private *dp = netdev_priv(dev);
@@ -230,41 +221,19 @@ static int ifb_open(struct net_device *dev)
return 0;
}
-static int ifb_newlink(struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[])
-{
- struct ifb_private *priv = netdev_priv(dev);
- int err;
-
- err = register_netdevice(dev);
- if (err < 0)
- return err;
-
- priv->dev = dev;
- list_add_tail(&priv->list, &ifbs);
- return 0;
-}
-
-static void ifb_dellink(struct net_device *dev)
-{
- struct ifb_private *priv = netdev_priv(dev);
-
- list_del(&priv->list);
- unregister_netdevice(dev);
-}
-
static struct rtnl_link_ops ifb_link_ops __read_mostly = {
.kind = "ifb",
.priv_size = sizeof(struct ifb_private),
.setup = ifb_setup,
- .newlink = ifb_newlink,
- .dellink = ifb_dellink,
};
+/* Number of ifb devices to be set up by this module. */
+module_param(numifbs, int, 0);
+MODULE_PARM_DESC(numifbs, "Number of ifb devices");
+
static int __init ifb_init_one(int index)
{
struct net_device *dev_ifb;
- struct ifb_private *priv;
int err;
dev_ifb = alloc_netdev(sizeof(struct ifb_private),
@@ -281,10 +250,6 @@ static int __init ifb_init_one(int index)
err = register_netdevice(dev_ifb);
if (err < 0)
goto err;
-
- priv = netdev_priv(dev_ifb);
- priv->dev = dev_ifb;
- list_add_tail(&priv->list, &ifbs);
return 0;
err:
@@ -294,7 +259,6 @@ err:
static int __init ifb_init_module(void)
{
- struct ifb_private *priv, *next;
int i, err;
rtnl_lock();
@@ -302,11 +266,8 @@ static int __init ifb_init_module(void)
for (i = 0; i < numifbs && !err; i++)
err = ifb_init_one(i);
- if (err) {
- list_for_each_entry_safe(priv, next, &ifbs, list)
- ifb_dellink(priv->dev);
+ if (err)
__rtnl_link_unregister(&ifb_link_ops);
- }
rtnl_unlock();
return err;
@@ -314,14 +275,7 @@ static int __init ifb_init_module(void)
static void __exit ifb_cleanup_module(void)
{
- struct ifb_private *priv, *next;
-
- rtnl_lock();
- list_for_each_entry_safe(priv, next, &ifbs, list)
- ifb_dellink(priv->dev);
-
- __rtnl_link_unregister(&ifb_link_ops);
- rtnl_unlock();
+ rtnl_link_unregister(&ifb_link_ops);
}
module_init(ifb_init_module);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index c5e5655..ec0c58a 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -115,26 +115,6 @@ err1:
return err;
}
-/* Cleanup all vlan devices
- * Note: devices that have been registered that but not
- * brought up will exist but have no module ref count.
- */
-static void __exit vlan_cleanup_devices(void)
-{
- struct net_device *dev, *nxt;
-
- rtnl_lock();
- for_each_netdev_safe(dev, nxt) {
- if (dev->priv_flags & IFF_802_1Q_VLAN) {
- unregister_vlan_dev(VLAN_DEV_INFO(dev)->real_dev,
- VLAN_DEV_INFO(dev)->vlan_id);
-
- unregister_netdevice(dev);
- }
- }
- rtnl_unlock();
-}
-
/*
* Module 'remove' entry point.
* o delete /proc/net/router directory and static entries.
@@ -150,7 +130,6 @@ static void __exit vlan_cleanup_module(void)
unregister_netdevice_notifier(&vlan_notifier_block);
dev_remove_pack(&vlan_packet_type);
- vlan_cleanup_devices();
/* This table must be empty if there are no module
* references left.
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 54c17e4..7b6b163 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -270,6 +270,9 @@ static LIST_HEAD(link_ops);
*/
int __rtnl_link_register(struct rtnl_link_ops *ops)
{
+ if (!ops->dellink)
+ ops->dellink = unregister_netdevice;
+
list_add_tail(&ops->list, &link_ops);
return 0;
}
@@ -298,12 +301,16 @@ EXPORT_SYMBOL_GPL(rtnl_link_register);
* __rtnl_link_unregister - Unregister rtnl_link_ops from rtnetlink.
* @ops: struct rtnl_link_ops * to unregister
*
- * The caller must hold the rtnl_mutex. This function should be used
- * by drivers that unregister devices during module unloading. It must
- * be called after unregistering the devices.
+ * The caller must hold the rtnl_mutex.
*/
void __rtnl_link_unregister(struct rtnl_link_ops *ops)
{
+ struct net_device *dev, *n;
+
+ for_each_netdev_safe(dev, n) {
+ if (dev->rtnl_link_ops == ops)
+ ops->dellink(dev);
+ }
list_del(&ops->list);
}
@@ -1067,7 +1074,10 @@ replay:
if (tb[IFLA_LINKMODE])
dev->link_mode = nla_get_u8(tb[IFLA_LINKMODE]);
- err = ops->newlink(dev, tb, data);
+ if (ops->newlink)
+ err = ops->newlink(dev, tb, data);
+ else
+ err = register_netdevice(dev);
err_free:
if (err < 0)
free_netdev(dev);
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RTNETLINK 03/04]: rtnl_link API simplification
2007-07-11 17:38 ` [RTNETLINK 03/04]: rtnl_link API simplification Patrick McHardy
@ 2007-07-12 2:42 ` David Miller
0 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2007-07-12 2:42 UTC (permalink / raw)
To: kaber; +Cc: netdev, greearb, xemul
From: Patrick McHardy <kaber@trash.net>
Date: Wed, 11 Jul 2007 19:38:17 +0200 (MEST)
> [RTNETLINK]: rtnl_link API simplification
>
> All drivers need to unregister their devices in the module unload function.
> While doing so they must hold the rtnl and atomically unregister the
> rtnl_link ops as well. This makes the rtnl_link_unregister function that
> takes the rtnl itself completely useless.
>
> Provide default newlink/dellink functions, make __rtnl_link_unregister and
> rtnl_link_unregister unregister all devices with matching rtnl_link_ops and
> change the existing users to take advantage of that.
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
Applied.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RTNETLINK 04/04]: rtnl_link: allow specifying initial device address
2007-07-11 17:38 [RTNETLINK 00/04]: rtnl_link improvements Patrick McHardy
` (2 preceding siblings ...)
2007-07-11 17:38 ` [RTNETLINK 03/04]: rtnl_link API simplification Patrick McHardy
@ 2007-07-11 17:38 ` Patrick McHardy
2007-07-12 2:43 ` David Miller
3 siblings, 1 reply; 13+ messages in thread
From: Patrick McHardy @ 2007-07-11 17:38 UTC (permalink / raw)
To: davem; +Cc: netdev, greearb, Patrick McHardy, xemul
[RTNETLINK]: rtnl_link: allow specifying initial device address
Drivers need to validate the initial addresses in their netlink attribute
validation function or manually reject them if they can't support this.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 2b73b83f72018cea40557afa0bce0a1b40a717be
tree e1f492495fe9d12bcbc5d623704c2ee476ffb914
parent 88fcde1e75dbfad7e7210642c0364850eed2591e
author Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:28:17 +0200
committer Patrick McHardy <kaber@trash.net> Wed, 11 Jul 2007 19:28:17 +0200
drivers/net/dummy.c | 12 ++++++++++++
drivers/net/ifb.c | 12 ++++++++++++
net/8021q/vlan.c | 8 ++++++--
net/8021q/vlan_netlink.c | 7 +++++++
net/core/rtnetlink.c | 9 +++++++--
5 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 373ff70..756a6bc 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -84,9 +84,21 @@ static int dummy_xmit(struct sk_buff *skb, struct net_device *dev)
return 0;
}
+static int dummy_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+ if (tb[IFLA_ADDRESS]) {
+ if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
+ return -EINVAL;
+ if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
+ return -EADDRNOTAVAIL;
+ }
+ return 0;
+}
+
static struct rtnl_link_ops dummy_link_ops __read_mostly = {
.kind = "dummy",
.setup = dummy_setup,
+ .validate = dummy_validate,
};
/* Number of dummy devices to be set up by this module. */
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index c8e7c8f..f5c3598 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -221,10 +221,22 @@ static int ifb_open(struct net_device *dev)
return 0;
}
+static int ifb_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+ if (tb[IFLA_ADDRESS]) {
+ if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
+ return -EINVAL;
+ if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
+ return -EADDRNOTAVAIL;
+ }
+ return 0;
+}
+
static struct rtnl_link_ops ifb_link_ops __read_mostly = {
.kind = "ifb",
.priv_size = sizeof(struct ifb_private),
.setup = ifb_setup,
+ .validate = ifb_validate,
};
/* Number of ifb devices to be set up by this module. */
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index ec0c58a..1a5e0f2 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -324,8 +324,10 @@ static int vlan_dev_init(struct net_device *dev)
(1<<__LINK_STATE_DORMANT))) |
(1<<__LINK_STATE_PRESENT);
- memcpy(dev->broadcast, real_dev->broadcast, real_dev->addr_len);
- memcpy(dev->dev_addr, real_dev->dev_addr, real_dev->addr_len);
+ if (is_zero_ether_addr(dev->dev_addr))
+ memcpy(dev->dev_addr, real_dev->dev_addr, dev->addr_len);
+ if (is_zero_ether_addr(dev->broadcast))
+ memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len);
if (real_dev->features & NETIF_F_HW_VLAN_TX) {
dev->hard_header = real_dev->hard_header;
@@ -373,6 +375,8 @@ void vlan_setup(struct net_device *new_dev)
new_dev->set_multicast_list = vlan_dev_set_multicast_list;
new_dev->destructor = free_netdev;
new_dev->do_ioctl = vlan_dev_ioctl;
+
+ memset(new_dev->broadcast, 0, sizeof(ETH_ALEN));
}
static void vlan_transfer_operstate(const struct net_device *dev, struct net_device *vlandev)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 844c7e4..6cdd1e0 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -41,6 +41,13 @@ static int vlan_validate(struct nlattr *tb[], struct nlattr *data[])
u16 id;
int err;
+ if (tb[IFLA_ADDRESS]) {
+ if (nla_len(tb[IFLA_ADDRESS]) != ETH_ALEN)
+ return -EINVAL;
+ if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
+ return -EADDRNOTAVAIL;
+ }
+
if (!data)
return -EINVAL;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 7b6b163..864cbdf 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1032,8 +1032,7 @@ replay:
if (ifm->ifi_index || ifm->ifi_flags || ifm->ifi_change)
return -EOPNOTSUPP;
- if (tb[IFLA_ADDRESS] || tb[IFLA_BROADCAST] || tb[IFLA_MAP] ||
- tb[IFLA_MASTER] || tb[IFLA_PROTINFO])
+ if (tb[IFLA_MAP] || tb[IFLA_MASTER] || tb[IFLA_PROTINFO])
return -EOPNOTSUPP;
if (!ops) {
@@ -1065,6 +1064,12 @@ replay:
if (tb[IFLA_MTU])
dev->mtu = nla_get_u32(tb[IFLA_MTU]);
+ if (tb[IFLA_ADDRESS])
+ memcpy(dev->dev_addr, nla_data(tb[IFLA_ADDRESS]),
+ nla_len(tb[IFLA_ADDRESS]));
+ if (tb[IFLA_BROADCAST])
+ memcpy(dev->broadcast, nla_data(tb[IFLA_BROADCAST]),
+ nla_len(tb[IFLA_BROADCAST]));
if (tb[IFLA_TXQLEN])
dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
if (tb[IFLA_WEIGHT])
^ permalink raw reply related [flat|nested] 13+ messages in thread