Re: [PATCH 1/2] Interface group: core (netlink) part

[Laszlo Attila Toth]

Patrick McHardy írta:

Laszlo Attila Toth wrote:
@@ -846,6 +850,12 @@ static int do_setlink(struct net_device *dev, 
struct ifinfomsg *ifm,

 write_unlock_bh(&dev_base_lock);
 }
 
+if (tb[IFLA_IFGROUP]) {

+write_lock_bh(&dev_base_lock);
+dev->ifgroup = nla_get_u32(tb[IFLA_IFGROUP]);
+write_unlock_bh(&dev_base_lock);
+}



So no notifications at all? Mhh .. I guess its OK for now, this stuff
needs to be fixed to notify once for all changes anyway.



I'll resend the following patch with the new ifgroup patches. The 
current changes: changes are now atomic and a notification will be send 
later at the end of the function. If an address was changed, a 
notification will be send and after that another which is always sent 
when anyi modification happened.


This patch may not be complete.


diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 4756d58..87ab3ff 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -819,6 +819,7 @@ static int do_setlink(struct net_device *dev, struct 
ifinfomsg *ifm,

if (tb[IFLA_BROADCAST]) {
nla_memcpy(dev->broadcast, tb[IFLA_BROADCAST], 
dev->addr_len);

send_addr_notify = 1;
+   modified = 1;
}

if (ifm->ifi_flags || ifm->ifi_change) {
@@ -829,21 +830,35 @@ static int do_setlink(struct net_device *dev, 
struct ifinfomsg *ifm,

flags = (flags & ifm->ifi_change) |
(dev->flags & ~ifm->ifi_change);
dev_change_flags(dev, flags);
+   modified = 1;
}

-   if (tb[IFLA_TXQLEN])
+   if (tb[IFLA_TXQLEN]) {
+   write_lock_bh(&dev_base_lock);
dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
+   write_unlock_bh(&dev_base_lock);
+   modified = 1;
+   }

-   if (tb[IFLA_WEIGHT])
+   if (tb[IFLA_WEIGHT]) {
+   write_lock_bh(&dev_base_lock);
dev->weight = nla_get_u32(tb[IFLA_WEIGHT]);
+   write_unlock_bh(&dev_base_lock);
+   modified = 1;
+   }

-   if (tb[IFLA_OPERSTATE])
+   if (tb[IFLA_OPERSTATE]) {
+   write_lock_bh(&dev_base_lock);
set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]));
+   write_unlock_bh(&dev_base_lock);
+   modified = 1;
+   }

if (tb[IFLA_LINKMODE]) {
write_lock_bh(&dev_base_lock);
dev->link_mode = nla_get_u8(tb[IFLA_LINKMODE]);
write_unlock_bh(&dev_base_lock);
+   modified = 1;
}

err = 0;
@@ -857,6 +872,8 @@ errout:

if (send_addr_notify)
call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+   if (modified)
+   rtmsg_ifinfo(RTM_NEWLINK, dev, 0)
return err;
 }

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Interface group: core (netlink) part

[Patrick McHardy]

Laszlo Attila Toth wrote:

@@ -846,6 +850,12 @@ static int do_setlink(struct net_device *dev, struct 
ifinfomsg *ifm,
write_unlock_bh(&dev_base_lock);
}
 
+	if (tb[IFLA_IFGROUP]) {

+   write_lock_bh(&dev_base_lock);
+   dev->ifgroup = nla_get_u32(tb[IFLA_IFGROUP]);
+   write_unlock_bh(&dev_base_lock);
+   }



So no notifications at all? Mhh .. I guess its OK for now, this stuff
needs to be fixed to notify once for all changes anyway.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Interface group: core (netlink) part

[Patrick McHardy]

Laszlo Attila Toth wrote:

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 4756d58..18ff3c5 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -646,6 +646,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct 
net_device *dev,
if (dev->master)
NLA_PUT_U32(skb, IFLA_MASTER, dev->master->ifindex);
 
+	if (dev->ifgroup)

+   NLA_PUT_U32(skb, IFLA_IFGROUP, dev->ifgroup);
+


You need to adjust if_nlmsg_size() to account for this.


if (dev->qdisc_sleeping)
NLA_PUT_STRING(skb, IFLA_QDISC, dev->qdisc_sleeping->ops->id);
 
@@ -846,6 +849,11 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,

write_unlock_bh(&dev_base_lock);
}
 
+	if (tb[IFLA_IFGROUP]) {

+   dev->ifgroup = nla_get_u32(tb[IFLA_IFGROUP ]);
+   rtmsg_ifinfo(RTM_NEWLINK, dev, 0);
+   }



Notifcations should be atomic, not one for every single change. I know
thats not the case currently, but it seems we're missing a notifcation
for some of the attributes anyway (like tx_queue_len, operstate,
linkmode), please consolidate all these and send a single notifcation.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html