* [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-09 20:48 [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications Thomas Graf
@ 2006-08-08 22:00 ` Thomas Graf
2006-08-10 15:51 ` Alexey Kuznetsov
2006-08-08 22:00 ` [PATCH 2/4] [RTNETLINK]: Use rtnl_multicast()/rtnl_unicast() Thomas Graf
` (3 subsequent siblings)
4 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-08 22:00 UTC (permalink / raw)
To: davem; +Cc: netdev
[-- Attachment #1: nl_echo --]
[-- Type: text/plain, Size: 1839 bytes --]
The current behaviour around NLM_F_ECHO is inconsistent
and spread all around in various subsystems and doesn't
represent what it was meant for.
This patch handles NLM_F_ECHO in netlink_rcv_skb() to
handle it in a central point. Most subsystems currently
interpret NLM_F_ECHO as to just unicast events to the
originator of the change while the real meaning of the
flag is to echo the request.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6.19.git/net/netlink/af_netlink.c
===================================================================
--- net-2.6.19.git.orig/net/netlink/af_netlink.c
+++ net-2.6.19.git/net/netlink/af_netlink.c
@@ -1430,6 +1430,28 @@ int netlink_dump_start(struct sock *ssk,
return 0;
}
+static void netlink_echo(struct sk_buff *in_skb, struct nlmsghdr *orig)
+{
+ struct sk_buff *skb;
+ struct nlmsghdr *nlh;
+ int payload = nlmsg_len(orig);
+
+ skb = nlmsg_new(nlmsg_total_size(payload), GFP_KERNEL);
+ if (skb == NULL)
+ return;
+
+ /*
+ * Return original header with pid set to 0 (from kernel) and
+ * NLM_F_REQUEST flag removed to indicate this is not a request
+ * for an echo but the reply.
+ */
+ nlh = __nlmsg_put(skb, 0, orig->nlmsg_seq, orig->nlmsg_type,
+ payload, orig->nlmsg_flags & ~NLM_F_REQUEST);
+ memcpy(nlmsg_data(nlh), nlmsg_data(orig), payload);
+
+ nlmsg_unicast(in_skb->sk, skb, NETLINK_CB(in_skb).pid);
+}
+
void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err)
{
struct sk_buff *skb;
@@ -1476,6 +1498,9 @@ static int netlink_rcv_skb(struct sk_buf
if (nlh->nlmsg_len < NLMSG_HDRLEN || skb->len < nlh->nlmsg_len)
return 0;
+ if (nlh->nlmsg_flags & NLM_F_ECHO)
+ netlink_echo(skb, nlh);
+
if (cb(skb, nlh, &err) < 0) {
/* Not an error, but we have to interrupt processing
* here. Note: that in this case we do not pull
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/4] [RTNETLINK]: Use rtnl_multicast()/rtnl_unicast()
2006-08-09 20:48 [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications Thomas Graf
2006-08-08 22:00 ` [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb() Thomas Graf
@ 2006-08-08 22:00 ` Thomas Graf
2006-08-09 21:00 ` [RESEND " Thomas Graf
2006-08-08 22:00 ` [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications Thomas Graf
` (2 subsequent siblings)
4 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-08 22:00 UTC (permalink / raw)
To: davem; +Cc: netdev
[-- Attachment #1: nl_msg --]
[-- Type: text/plain, Size: 17565 bytes --]
Introduces rtnl_multicast() to broadcast events and rtnl_unicast()
to send unicast messages via the rtnl socket.
This obsoletes rtnetlink_send() which wrongly provided the netlink
pid of the requesting applications to netlink_broadcast() which
lead to events not being sent to that application even though it
was subscribed.
This patch ensures consistency in all rtnetlink event notification
mechansims, i.e. the pid of the requesting application is kept
in nlmsg_pid to help applications such as quagga to identify whether
the event was caused by themselves and properly feeds all calls
to netlink_broadcast() with a pid of 0 to not exclude any sockets.
Removes the obsoleted NLM_F_ECHO code.
Some notifications are done in atomic contex, therefore
nlmsg_multicast() had to be extended to take allocation flags.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6.19.git/include/linux/rtnetlink.h
===================================================================
--- net-2.6.19.git.orig/include/linux/rtnetlink.h
+++ net-2.6.19.git/include/linux/rtnetlink.h
@@ -583,7 +583,8 @@ struct rtnetlink_link
};
extern struct rtnetlink_link * rtnetlink_links[NPROTO];
-extern int rtnetlink_send(struct sk_buff *skb, u32 pid, u32 group, int echo);
+extern int rtnl_multicast(struct sk_buff *skb, unsigned int group, gfp_t flags);
+extern int rtnl_unicast(struct sk_buff *skb, u32 pid);
extern int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics);
extern void __rta_fill(struct sk_buff *skb, int attrtype, int attrlen, const void *data);
Index: net-2.6.19.git/net/core/rtnetlink.c
===================================================================
--- net-2.6.19.git.orig/net/core/rtnetlink.c
+++ net-2.6.19.git/net/core/rtnetlink.c
@@ -153,17 +153,14 @@ size_t rtattr_strlcpy(char *dest, const
return ret;
}
-int rtnetlink_send(struct sk_buff *skb, u32 pid, unsigned group, int echo)
+int rtnl_multicast(struct sk_buff *skb, unsigned int group, gfp_t flags)
{
- int err = 0;
+ return nlmsg_multicast(rtnl, skb, 0, group, flags);
+}
- NETLINK_CB(skb).dst_group = group;
- if (echo)
- atomic_inc(&skb->users);
- netlink_broadcast(rtnl, skb, pid, group, GFP_KERNEL);
- if (echo)
- err = netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
- return err;
+int rtnl_unicast(struct sk_buff *skb, u32 pid)
+{
+ return nlmsg_unicast(rtnl, skb, pid);
}
int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics)
@@ -560,9 +557,7 @@ static int rtnl_getlink(struct sk_buff *
goto errout;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(skb).pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
+ err = rtnl_unicast(skb, NETLINK_CB(skb).pid);
errout:
kfree(iw_buf);
dev_put(dev);
@@ -609,8 +604,8 @@ void rtmsg_ifinfo(int type, struct net_d
kfree_skb(skb);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_KERNEL);
+
+ rtnl_multicast(skb, RTNLGRP_LINK, GFP_KERNEL);
}
/* Protected by RTNL sempahore. */
Index: net-2.6.19.git/net/decnet/dn_table.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_table.c
+++ net-2.6.19.git/net/decnet/dn_table.c
@@ -343,12 +343,8 @@ static void dn_rtmsg_fib(int event, stru
kfree_skb(skb);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_DECnet_ROUTE;
- if (nlh->nlmsg_flags & NLM_F_ECHO)
- atomic_inc(&skb->users);
- netlink_broadcast(rtnl, skb, pid, RTNLGRP_DECnet_ROUTE, GFP_KERNEL);
- if (nlh->nlmsg_flags & NLM_F_ECHO)
- netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
+
+ rtnl_multicast(skb, RTNLGRP_DECnet_ROUTE, GFP_KERNEL);
}
static __inline__ int dn_hash_dump_bucket(struct sk_buff *skb,
Index: net-2.6.19.git/net/ipv4/fib_semantics.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/fib_semantics.c
+++ net-2.6.19.git/net/ipv4/fib_semantics.c
@@ -291,12 +291,8 @@ void rtmsg_fib(int event, u32 key, struc
kfree_skb(skb);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV4_ROUTE;
- if (n->nlmsg_flags&NLM_F_ECHO)
- atomic_inc(&skb->users);
- netlink_broadcast(rtnl, skb, pid, RTNLGRP_IPV4_ROUTE, GFP_KERNEL);
- if (n->nlmsg_flags&NLM_F_ECHO)
- netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
+
+ rtnl_multicast(skb, RTNLGRP_IPV4_ROUTE, GFP_KERNEL);
}
/* Return the first fib alias matching TOS with
Index: net-2.6.19.git/net/sched/act_api.c
===================================================================
--- net-2.6.19.git.orig/net/sched/act_api.c
+++ net-2.6.19.git/net/sched/act_api.c
@@ -459,7 +459,6 @@ static int
act_get_notify(u32 pid, struct nlmsghdr *n, struct tc_action *a, int event)
{
struct sk_buff *skb;
- int err = 0;
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
@@ -468,10 +467,8 @@ act_get_notify(u32 pid, struct nlmsghdr
kfree_skb(skb);
return -EINVAL;
}
- err = netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
- return err;
+
+ return rtnl_unicast(skb, pid);
}
static struct tc_action *
@@ -592,11 +589,8 @@ static int tca_action_flush(struct rtatt
nlh->nlmsg_flags |= NLM_F_ROOT;
module_put(a->ops->owner);
kfree(a);
- err = rtnetlink_send(skb, pid, RTNLGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
- if (err > 0)
- return 0;
- return err;
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
rtattr_failure:
nlmsg_failure:
@@ -655,11 +649,8 @@ tca_action_gd(struct rtattr *rta, struct
/* now do the delete */
tcf_action_destroy(head, 0);
- ret = rtnetlink_send(skb, pid, RTNLGRP_TC,
- n->nlmsg_flags&NLM_F_ECHO);
- if (ret > 0)
- return 0;
- return ret;
+
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
}
err:
cleanup_a(head);
@@ -674,7 +665,6 @@ static int tcf_add_notify(struct tc_acti
struct sk_buff *skb;
struct rtattr *x;
unsigned char *b;
- int err = 0;
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
@@ -697,12 +687,8 @@ static int tcf_add_notify(struct tc_acti
x->rta_len = skb->tail - (u8*)x;
nlh->nlmsg_len = skb->tail - b;
- NETLINK_CB(skb).dst_group = RTNLGRP_TC;
- err = rtnetlink_send(skb, pid, RTNLGRP_TC, flags&NLM_F_ECHO);
- if (err > 0)
- err = 0;
- return err;
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
rtattr_failure:
nlmsg_failure:
Index: net-2.6.19.git/net/sched/cls_api.c
===================================================================
--- net-2.6.19.git.orig/net/sched/cls_api.c
+++ net-2.6.19.git/net/sched/cls_api.c
@@ -366,7 +366,7 @@ static int tfilter_notify(struct sk_buff
return -EINVAL;
}
- return rtnetlink_send(skb, pid, RTNLGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
}
struct tcf_dump_args
Index: net-2.6.19.git/net/sched/sch_api.c
===================================================================
--- net-2.6.19.git.orig/net/sched/sch_api.c
+++ net-2.6.19.git/net/sched/sch_api.c
@@ -815,7 +815,7 @@ static int qdisc_notify(struct sk_buff *
}
if (skb->len)
- return rtnetlink_send(skb, pid, RTNLGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
err_out:
kfree_skb(skb);
@@ -1039,7 +1039,7 @@ static int tclass_notify(struct sk_buff
return -EINVAL;
}
- return rtnetlink_send(skb, pid, RTNLGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
}
struct qdisc_dump_args
Index: net-2.6.19.git/include/net/genetlink.h
===================================================================
--- net-2.6.19.git.orig/include/net/genetlink.h
+++ net-2.6.19.git/include/net/genetlink.h
@@ -133,11 +133,12 @@ static inline int genlmsg_cancel(struct
* @skb: netlink message as socket buffer
* @pid: own netlink pid to avoid sending to yourself
* @group: multicast group id
+ * @flags: allocation flags
*/
static inline int genlmsg_multicast(struct sk_buff *skb, u32 pid,
- unsigned int group)
+ unsigned int group, gfp_t flags)
{
- return nlmsg_multicast(genl_sock, skb, pid, group);
+ return nlmsg_multicast(genl_sock, skb, pid, group, flags);
}
/**
Index: net-2.6.19.git/include/net/netlink.h
===================================================================
--- net-2.6.19.git.orig/include/net/netlink.h
+++ net-2.6.19.git/include/net/netlink.h
@@ -545,15 +545,16 @@ static inline void nlmsg_free(struct sk_
* @skb: netlink message as socket buffer
* @pid: own netlink pid to avoid sending to yourself
* @group: multicast group id
+ * @flags: allocation flags
*/
static inline int nlmsg_multicast(struct sock *sk, struct sk_buff *skb,
- u32 pid, unsigned int group)
+ u32 pid, unsigned int group, gfp_t flags)
{
int err;
NETLINK_CB(skb).dst_group = group;
- err = netlink_broadcast(sk, skb, pid, group, GFP_KERNEL);
+ err = netlink_broadcast(sk, skb, pid, group, flags);
if (err > 0)
err = 0;
Index: net-2.6.19.git/net/netlabel/netlabel_user.c
===================================================================
--- net-2.6.19.git.orig/net/netlabel/netlabel_user.c
+++ net-2.6.19.git/net/netlabel/netlabel_user.c
@@ -154,5 +154,5 @@ int netlbl_netlink_snd(struct sk_buff *s
*/
int netlbl_netlink_snd_multicast(struct sk_buff *skb, u32 pid, u32 group)
{
- return genlmsg_multicast(skb, pid, group);
+ return genlmsg_multicast(skb, pid, group, GFP_KERNEL);
}
Index: net-2.6.19.git/net/netlink/genetlink.c
===================================================================
--- net-2.6.19.git.orig/net/netlink/genetlink.c
+++ net-2.6.19.git/net/netlink/genetlink.c
@@ -510,7 +510,7 @@ static int genl_ctrl_event(int event, vo
if (IS_ERR(msg))
return PTR_ERR(msg);
- genlmsg_multicast(msg, 0, GENL_ID_CTRL);
+ genlmsg_multicast(msg, 0, GENL_ID_CTRL, GFP_KERNEL);
break;
}
Index: net-2.6.19.git/net/bridge/br_netlink.c
===================================================================
--- net-2.6.19.git.orig/net/bridge/br_netlink.c
+++ net-2.6.19.git/net/bridge/br_netlink.c
@@ -88,8 +88,7 @@ void br_ifinfo_notify(int event, struct
if (err)
goto err_kfree;
- NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_ATOMIC);
+ rtnl_multicast(skb, RTNLGRP_LINK, GFP_ATOMIC);
return;
err_kfree:
Index: net-2.6.19.git/net/core/fib_rules.c
===================================================================
--- net-2.6.19.git.orig/net/core/fib_rules.c
+++ net-2.6.19.git/net/core/fib_rules.c
@@ -354,7 +354,7 @@ static void notify_rule_change(int event
kfree_skb(skb);
netlink_set_err(rtnl, 0, ops->nlgroup, EINVAL);
} else
- netlink_broadcast(rtnl, skb, 0, ops->nlgroup, GFP_KERNEL);
+ rtnl_multicast(skb, ops->nlgroup, GFP_KERNEL);
}
static void attach_rules(struct list_head *rules, struct net_device *dev)
Index: net-2.6.19.git/net/core/neighbour.c
===================================================================
--- net-2.6.19.git.orig/net/core/neighbour.c
+++ net-2.6.19.git/net/core/neighbour.c
@@ -2416,10 +2416,8 @@ void neigh_app_ns(struct neighbour *n)
if (neigh_fill_info(skb, n, 0, 0, RTM_GETNEIGH, NLM_F_REQUEST) <= 0)
kfree_skb(skb);
- else {
- NETLINK_CB(skb).dst_group = RTNLGRP_NEIGH;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_NEIGH, GFP_ATOMIC);
- }
+ else
+ rtnl_multicast(skb, RTNLGRP_NEIGH, GFP_ATOMIC);
}
static void neigh_app_notify(struct neighbour *n)
@@ -2432,10 +2430,8 @@ static void neigh_app_notify(struct neig
if (neigh_fill_info(skb, n, 0, 0, RTM_NEWNEIGH, 0) <= 0)
kfree_skb(skb);
- else {
- NETLINK_CB(skb).dst_group = RTNLGRP_NEIGH;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_NEIGH, GFP_ATOMIC);
- }
+ else
+ rtnl_multicast(skb, RTNLGRP_NEIGH, GFP_ATOMIC);
}
#endif /* CONFIG_ARPD */
Index: net-2.6.19.git/net/core/wireless.c
===================================================================
--- net-2.6.19.git.orig/net/core/wireless.c
+++ net-2.6.19.git/net/core/wireless.c
@@ -1903,8 +1903,8 @@ static inline void rtmsg_iwinfo(struct n
kfree_skb(skb);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_ATOMIC);
+
+ rtnl_multicast(skb, RTNLGRP_LINK, GFP_ATOMIC);
}
#endif /* WE_EVENT_RTNETLINK */
Index: net-2.6.19.git/net/decnet/dn_dev.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_dev.c
+++ net-2.6.19.git/net/decnet/dn_dev.c
@@ -757,8 +757,8 @@ static void rtmsg_ifa(int event, struct
netlink_set_err(rtnl, 0, RTNLGRP_DECnet_IFADDR, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_DECnet_IFADDR;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_DECnet_IFADDR, GFP_KERNEL);
+
+ rtnl_multicast(skb, RTNLGRP_DECnet_IFADDR, GFP_KERNEL);
}
static int dn_dev_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
Index: net-2.6.19.git/net/decnet/dn_route.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_route.c
+++ net-2.6.19.git/net/decnet/dn_route.c
@@ -1609,9 +1609,7 @@ int dn_cache_getroute(struct sk_buff *in
goto out_free;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
-
- return err;
+ return rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
out_free:
kfree_skb(skb);
Index: net-2.6.19.git/net/ipv4/devinet.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/devinet.c
+++ net-2.6.19.git/net/ipv4/devinet.c
@@ -1200,7 +1200,7 @@ static void rtmsg_ifa(int event, struct
kfree_skb(skb);
netlink_set_err(rtnl, 0, RTNLGRP_IPV4_IFADDR, EINVAL);
} else
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV4_IFADDR, GFP_KERNEL);
+ rtnl_multicast(skb, RTNLGRP_IPV4_IFADDR, GFP_KERNEL);
}
static struct rtnetlink_link inet_rtnetlink_table[RTM_NR_MSGTYPES] = {
Index: net-2.6.19.git/net/ipv4/ipmr.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/ipmr.c
+++ net-2.6.19.git/net/ipv4/ipmr.c
@@ -312,7 +312,7 @@ static void ipmr_destroy_unres(struct mf
e = NLMSG_DATA(nlh);
e->error = -ETIMEDOUT;
memset(&e->msg, 0, sizeof(e->msg));
- netlink_unicast(rtnl, skb, NETLINK_CB(skb).dst_pid, MSG_DONTWAIT);
+ rtnl_unicast(skb, NETLINK_CB(skb).dst_pid);
} else
kfree_skb(skb);
}
@@ -525,7 +525,7 @@ static void ipmr_cache_resolve(struct mf
e->error = -EMSGSIZE;
memset(&e->msg, 0, sizeof(e->msg));
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(skb).dst_pid, MSG_DONTWAIT);
+ err = rtnl_unicast(skb, NETLINK_CB(skb).dst_pid);
} else
ip_mr_forward(skb, c, 0);
}
Index: net-2.6.19.git/net/ipv4/route.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/route.c
+++ net-2.6.19.git/net/ipv4/route.c
@@ -2808,9 +2808,7 @@ int inet_rtm_getroute(struct sk_buff *in
goto out_free;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
+ err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
out: return err;
out_free:
Index: net-2.6.19.git/net/ipv6/addrconf.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/addrconf.c
+++ net-2.6.19.git/net/ipv6/addrconf.c
@@ -3268,9 +3268,7 @@ static int inet6_rtm_getaddr(struct sk_b
goto out_free;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
+ err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
out:
in6_ifa_put(ifa);
return err;
@@ -3294,8 +3292,8 @@ static void inet6_ifa_notify(int event,
netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFADDR, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_IFADDR;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFADDR, GFP_ATOMIC);
+
+ rtnl_multicast(skb, RTNLGRP_IPV6_IFADDR, GFP_ATOMIC);
}
static void inline ipv6_store_devconf(struct ipv6_devconf *cnf,
@@ -3448,8 +3446,8 @@ void inet6_ifinfo_notify(int event, stru
netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFINFO, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_IFINFO;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
+
+ rtnl_multicast(skb, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
}
/* Maximum length of prefix_cacheinfo attributes */
@@ -3513,8 +3511,8 @@ static void inet6_prefix_notify(int even
netlink_set_err(rtnl, 0, RTNLGRP_IPV6_PREFIX, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_PREFIX;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_PREFIX, GFP_ATOMIC);
+
+ rtnl_multicast(skb, RTNLGRP_IPV6_PREFIX, GFP_ATOMIC);
}
static struct rtnetlink_link inet6_rtnetlink_table[RTM_NR_MSGTYPES] = {
Index: net-2.6.19.git/net/ipv6/route.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/route.c
+++ net-2.6.19.git/net/ipv6/route.c
@@ -2161,9 +2161,7 @@ int inet6_rtm_getroute(struct sk_buff *i
goto out_free;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
+ err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
out:
return err;
out_free:
@@ -2194,8 +2192,8 @@ void inet6_rt_notify(int event, struct r
netlink_set_err(rtnl, 0, RTNLGRP_IPV6_ROUTE, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_ROUTE;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_ROUTE, gfp_any());
+
+ rtnl_multicast(skb, RTNLGRP_IPV6_ROUTE, gfp_any());
}
/*
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications
2006-08-09 20:48 [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications Thomas Graf
2006-08-08 22:00 ` [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb() Thomas Graf
2006-08-08 22:00 ` [PATCH 2/4] [RTNETLINK]: Use rtnl_multicast()/rtnl_unicast() Thomas Graf
@ 2006-08-08 22:00 ` Thomas Graf
2006-08-10 18:09 ` Patrick McHardy
2006-08-08 22:00 ` [PATCH 4/4] [RTNETLINK]: Unexport global rtnl sock Thomas Graf
2006-08-10 4:16 ` [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications David Miller
4 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-08 22:00 UTC (permalink / raw)
To: davem; +Cc: netdev
[-- Attachment #1: nl_remove_set_err --]
[-- Type: text/plain, Size: 8930 bytes --]
Setting a socket error on all sockets subscribed to a group
if an event notificiation of said group fails due to memory
pressure only confuses applications and is of no use.
This patch removes it all together.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6.19.git/net/bridge/br_netlink.c
===================================================================
--- net-2.6.19.git.orig/net/bridge/br_netlink.c
+++ net-2.6.19.git/net/bridge/br_netlink.c
@@ -76,25 +76,17 @@ rtattr_failure:
void br_ifinfo_notify(int event, struct net_bridge_port *port)
{
struct sk_buff *skb;
- int err = -ENOMEM;
pr_debug("bridge notify event=%d\n", event);
skb = alloc_skb(NLMSG_SPACE(sizeof(struct ifinfomsg) + 128),
GFP_ATOMIC);
if (!skb)
- goto err_out;
+ return;
- err = br_fill_ifinfo(skb, port, current->pid, 0, event, 0);
- if (err)
- goto err_kfree;
-
- rtnl_multicast(skb, RTNLGRP_LINK, GFP_ATOMIC);
- return;
-
-err_kfree:
- kfree_skb(skb);
-err_out:
- netlink_set_err(rtnl, 0, RTNLGRP_LINK, err);
+ if (br_fill_ifinfo(skb, port, current->pid, 0, event, 0) <= 0)
+ kfree_skb(skb);
+ else
+ rtnl_multicast(skb, RTNLGRP_LINK, GFP_ATOMIC);
}
/*
Index: net-2.6.19.git/net/core/fib_rules.c
===================================================================
--- net-2.6.19.git.orig/net/core/fib_rules.c
+++ net-2.6.19.git/net/core/fib_rules.c
@@ -349,11 +349,11 @@ static void notify_rule_change(int event
struct sk_buff *skb = alloc_skb(size, GFP_KERNEL);
if (skb == NULL)
- netlink_set_err(rtnl, 0, ops->nlgroup, ENOBUFS);
- else if (fib_nl_fill_rule(skb, rule, 0, 0, event, 0, ops) < 0) {
+ return;
+
+ if (fib_nl_fill_rule(skb, rule, 0, 0, event, 0, ops) <= 0)
kfree_skb(skb);
- netlink_set_err(rtnl, 0, ops->nlgroup, EINVAL);
- } else
+ else
rtnl_multicast(skb, ops->nlgroup, GFP_KERNEL);
}
Index: net-2.6.19.git/net/decnet/dn_dev.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_dev.c
+++ net-2.6.19.git/net/decnet/dn_dev.c
@@ -748,17 +748,13 @@ static void rtmsg_ifa(int event, struct
int size = NLMSG_SPACE(sizeof(struct ifaddrmsg)+128);
skb = alloc_skb(size, GFP_KERNEL);
- if (!skb) {
- netlink_set_err(rtnl, 0, RTNLGRP_DECnet_IFADDR, ENOBUFS);
- return;
- }
- if (dn_dev_fill_ifaddr(skb, ifa, 0, 0, event, 0) < 0) {
- kfree_skb(skb);
- netlink_set_err(rtnl, 0, RTNLGRP_DECnet_IFADDR, EINVAL);
+ if (skb == NULL)
return;
- }
- rtnl_multicast(skb, RTNLGRP_DECnet_IFADDR, GFP_KERNEL);
+ if (dn_dev_fill_ifaddr(skb, ifa, 0, 0, event, 0) <= 0)
+ kfree_skb(skb);
+ else
+ rtnl_multicast(skb, RTNLGRP_DECnet_IFADDR, GFP_KERNEL);
}
static int dn_dev_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
Index: net-2.6.19.git/net/ipv4/devinet.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/devinet.c
+++ net-2.6.19.git/net/ipv4/devinet.c
@@ -1195,11 +1195,11 @@ static void rtmsg_ifa(int event, struct
skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
if (skb == NULL)
- netlink_set_err(rtnl, 0, RTNLGRP_IPV4_IFADDR, ENOBUFS);
- else if (inet_fill_ifaddr(skb, ifa, 0, 0, event, 0) < 0) {
+ return;
+
+ if (inet_fill_ifaddr(skb, ifa, 0, 0, event, 0) <= 0)
kfree_skb(skb);
- netlink_set_err(rtnl, 0, RTNLGRP_IPV4_IFADDR, EINVAL);
- } else
+ else
rtnl_multicast(skb, RTNLGRP_IPV4_IFADDR, GFP_KERNEL);
}
Index: net-2.6.19.git/net/ipv6/addrconf.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/addrconf.c
+++ net-2.6.19.git/net/ipv6/addrconf.c
@@ -3283,17 +3283,13 @@ static void inet6_ifa_notify(int event,
int size = NLMSG_SPACE(sizeof(struct ifaddrmsg) + INET6_IFADDR_RTA_SPACE);
skb = alloc_skb(size, GFP_ATOMIC);
- if (!skb) {
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFADDR, ENOBUFS);
+ if (skb == NULL)
return;
- }
- if (inet6_fill_ifaddr(skb, ifa, current->pid, 0, event, 0) < 0) {
- kfree_skb(skb);
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFADDR, EINVAL);
- return;
- }
- rtnl_multicast(skb, RTNLGRP_IPV6_IFADDR, GFP_ATOMIC);
+ if (inet6_fill_ifaddr(skb, ifa, current->pid, 0, event, 0) <= 0)
+ kfree_skb(skb);
+ else
+ rtnl_multicast(skb, RTNLGRP_IPV6_IFADDR, GFP_ATOMIC);
}
static void inline ipv6_store_devconf(struct ipv6_devconf *cnf,
@@ -3437,17 +3433,13 @@ void inet6_ifinfo_notify(int event, stru
int size = NLMSG_SPACE(sizeof(struct ifinfomsg) + INET6_IFINFO_RTA_SPACE);
skb = alloc_skb(size, GFP_ATOMIC);
- if (!skb) {
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFINFO, ENOBUFS);
- return;
- }
- if (inet6_fill_ifinfo(skb, idev, current->pid, 0, event, 0) < 0) {
- kfree_skb(skb);
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFINFO, EINVAL);
+ if (skb == NULL)
return;
- }
- rtnl_multicast(skb, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
+ if (inet6_fill_ifinfo(skb, idev, current->pid, 0, event, 0) <= 0)
+ kfree_skb(skb);
+ else
+ rtnl_multicast(skb, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
}
/* Maximum length of prefix_cacheinfo attributes */
@@ -3502,17 +3494,13 @@ static void inet6_prefix_notify(int even
int size = NLMSG_SPACE(sizeof(struct prefixmsg) + INET6_PREFIX_RTA_SPACE);
skb = alloc_skb(size, GFP_ATOMIC);
- if (!skb) {
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_PREFIX, ENOBUFS);
+ if (skb == NULL)
return;
- }
- if (inet6_fill_prefix(skb, idev, pinfo, current->pid, 0, event, 0) < 0) {
- kfree_skb(skb);
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_PREFIX, EINVAL);
- return;
- }
- rtnl_multicast(skb, RTNLGRP_IPV6_PREFIX, GFP_ATOMIC);
+ if (inet6_fill_prefix(skb, idev, pinfo, current->pid, 0, event, 0) <= 0)
+ kfree_skb(skb);
+ else
+ rtnl_multicast(skb, RTNLGRP_IPV6_PREFIX, GFP_ATOMIC);
}
static struct rtnetlink_link inet6_rtnetlink_table[RTM_NR_MSGTYPES] = {
Index: net-2.6.19.git/net/ipv6/route.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/route.c
+++ net-2.6.19.git/net/ipv6/route.c
@@ -2183,17 +2183,13 @@ void inet6_rt_notify(int event, struct r
seq = nlh->nlmsg_seq;
skb = alloc_skb(size, gfp_any());
- if (!skb) {
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_ROUTE, ENOBUFS);
+ if (skb == NULL)
return;
- }
- if (rt6_fill_node(skb, rt, NULL, NULL, 0, event, pid, seq, 0, 0) < 0) {
- kfree_skb(skb);
- netlink_set_err(rtnl, 0, RTNLGRP_IPV6_ROUTE, EINVAL);
- return;
- }
- rtnl_multicast(skb, RTNLGRP_IPV6_ROUTE, gfp_any());
+ if (rt6_fill_node(skb, rt, NULL, NULL, 0, event, pid, seq, 0, 0) <= 0)
+ kfree_skb(skb);
+ else
+ rtnl_multicast(skb, RTNLGRP_IPV6_ROUTE, gfp_any());
}
/*
Index: net-2.6.19.git/include/linux/netlink.h
===================================================================
--- net-2.6.19.git.orig/include/linux/netlink.h
+++ net-2.6.19.git/include/linux/netlink.h
@@ -156,7 +156,6 @@ extern int netlink_has_listeners(struct
extern int netlink_unicast(struct sock *ssk, struct sk_buff *skb, __u32 pid, int nonblock);
extern int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, __u32 pid,
__u32 group, gfp_t allocation);
-extern void netlink_set_err(struct sock *ssk, __u32 pid, __u32 group, int code);
extern int netlink_register_notifier(struct notifier_block *nb);
extern int netlink_unregister_notifier(struct notifier_block *nb);
Index: net-2.6.19.git/net/netlink/af_netlink.c
===================================================================
--- net-2.6.19.git.orig/net/netlink/af_netlink.c
+++ net-2.6.19.git/net/netlink/af_netlink.c
@@ -955,50 +955,6 @@ int netlink_broadcast(struct sock *ssk,
return -ESRCH;
}
-struct netlink_set_err_data {
- struct sock *exclude_sk;
- u32 pid;
- u32 group;
- int code;
-};
-
-static inline int do_one_set_err(struct sock *sk,
- struct netlink_set_err_data *p)
-{
- struct netlink_sock *nlk = nlk_sk(sk);
-
- if (sk == p->exclude_sk)
- goto out;
-
- if (nlk->pid == p->pid || p->group - 1 >= nlk->ngroups ||
- !test_bit(p->group - 1, nlk->groups))
- goto out;
-
- sk->sk_err = p->code;
- sk->sk_error_report(sk);
-out:
- return 0;
-}
-
-void netlink_set_err(struct sock *ssk, u32 pid, u32 group, int code)
-{
- struct netlink_set_err_data info;
- struct hlist_node *node;
- struct sock *sk;
-
- info.exclude_sk = ssk;
- info.pid = pid;
- info.group = group;
- info.code = code;
-
- read_lock(&nl_table_lock);
-
- sk_for_each_bound(sk, node, &nl_table[ssk->sk_protocol].mc_list)
- do_one_set_err(sk, &info);
-
- read_unlock(&nl_table_lock);
-}
-
static int netlink_setsockopt(struct socket *sock, int level, int optname,
char __user *optval, int optlen)
{
@@ -1825,7 +1781,6 @@ EXPORT_SYMBOL(netlink_broadcast);
EXPORT_SYMBOL(netlink_dump_start);
EXPORT_SYMBOL(netlink_kernel_create);
EXPORT_SYMBOL(netlink_register_notifier);
-EXPORT_SYMBOL(netlink_set_err);
EXPORT_SYMBOL(netlink_set_nonroot);
EXPORT_SYMBOL(netlink_unicast);
EXPORT_SYMBOL(netlink_unregister_notifier);
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/4] [RTNETLINK]: Unexport global rtnl sock
2006-08-09 20:48 [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications Thomas Graf
` (2 preceding siblings ...)
2006-08-08 22:00 ` [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications Thomas Graf
@ 2006-08-08 22:00 ` Thomas Graf
2006-08-10 4:16 ` [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications David Miller
4 siblings, 0 replies; 25+ messages in thread
From: Thomas Graf @ 2006-08-08 22:00 UTC (permalink / raw)
To: davem; +Cc: netdev
[-- Attachment #1: unexport_rtnl --]
[-- Type: text/plain, Size: 1433 bytes --]
All references to the rtnl sock are now done via wrapper
functions, unexport it.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6.19.git/include/linux/rtnetlink.h
===================================================================
--- net-2.6.19.git.orig/include/linux/rtnetlink.h
+++ net-2.6.19.git/include/linux/rtnetlink.h
@@ -574,8 +574,6 @@ extern int rtattr_parse(struct rtattr *t
#define rtattr_parse_nested(tb, max, rta) \
rtattr_parse((tb), (max), RTA_DATA((rta)), RTA_PAYLOAD((rta)))
-extern struct sock *rtnl;
-
struct rtnetlink_link
{
int (*doit)(struct sk_buff *, struct nlmsghdr*, void *attr);
Index: net-2.6.19.git/net/core/rtnetlink.c
===================================================================
--- net-2.6.19.git.orig/net/core/rtnetlink.c
+++ net-2.6.19.git/net/core/rtnetlink.c
@@ -58,6 +58,7 @@
#endif /* CONFIG_NET_WIRELESS_RTNETLINK */
static DEFINE_MUTEX(rtnl_mutex);
+static struct sock *rtnl;
void rtnl_lock(void)
{
@@ -95,8 +96,6 @@ int rtattr_parse(struct rtattr *tb[], in
return 0;
}
-struct sock *rtnl;
-
struct rtnetlink_link * rtnetlink_links[NPROTO];
static const int rtm_min[RTM_NR_FAMILIES] =
@@ -802,7 +801,6 @@ EXPORT_SYMBOL(rtattr_strlcpy);
EXPORT_SYMBOL(rtattr_parse);
EXPORT_SYMBOL(rtnetlink_links);
EXPORT_SYMBOL(rtnetlink_put_metrics);
-EXPORT_SYMBOL(rtnl);
EXPORT_SYMBOL(rtnl_lock);
EXPORT_SYMBOL(rtnl_trylock);
EXPORT_SYMBOL(rtnl_unlock);
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications
@ 2006-08-09 20:48 Thomas Graf
2006-08-08 22:00 ` [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb() Thomas Graf
` (4 more replies)
0 siblings, 5 replies; 25+ messages in thread
From: Thomas Graf @ 2006-08-09 20:48 UTC (permalink / raw)
To: davem; +Cc: netdev
Aims at cleaning up rtnetlink event notifications and implements
real NLM_F_ECHO support.
Please disregard my last IPv4 routing related patchset, I'll
resubmit it based on this patchset.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RESEND 2/4] [RTNETLINK]: Use rtnl_multicast()/rtnl_unicast()
2006-08-08 22:00 ` [PATCH 2/4] [RTNETLINK]: Use rtnl_multicast()/rtnl_unicast() Thomas Graf
@ 2006-08-09 21:00 ` Thomas Graf
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Graf @ 2006-08-09 21:00 UTC (permalink / raw)
To: davem; +Cc: netdev
... forgot to export the new symbols.
Introduces rtnl_multicast() to broadcast events and rtnl_unicast()
to send unicast messages via the rtnl socket.
This obsoletes rtnetlink_send() which wrongly provided the netlink
pid of the requesting applications to netlink_broadcast() which
lead to events not being sent to that application even though it
was subscribed.
This patch ensures consistency in all rtnetlink event notification
mechansims, i.e. the pid of the requesting application is kept
in nlmsg_pid to help applications such as quagga to identify whether
the event was caused by themselves and properly feeds all calls
to netlink_broadcast() with a pid of 0 to not exclude any sockets.
Removes the obsoleted NLM_F_ECHO code.
Some notifications are done in atomic contex, therefore
nlmsg_multicast() had to be extended to take allocation flags.
Signed-off-by: Thomas Graf <tgraf@suug.ch>
Index: net-2.6.19.git/include/linux/rtnetlink.h
===================================================================
--- net-2.6.19.git.orig/include/linux/rtnetlink.h
+++ net-2.6.19.git/include/linux/rtnetlink.h
@@ -583,7 +583,8 @@ struct rtnetlink_link
};
extern struct rtnetlink_link * rtnetlink_links[NPROTO];
-extern int rtnetlink_send(struct sk_buff *skb, u32 pid, u32 group, int echo);
+extern int rtnl_multicast(struct sk_buff *skb, unsigned int group, gfp_t flags);
+extern int rtnl_unicast(struct sk_buff *skb, u32 pid);
extern int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics);
extern void __rta_fill(struct sk_buff *skb, int attrtype, int attrlen, const void *data);
Index: net-2.6.19.git/net/core/rtnetlink.c
===================================================================
--- net-2.6.19.git.orig/net/core/rtnetlink.c
+++ net-2.6.19.git/net/core/rtnetlink.c
@@ -153,17 +153,14 @@ size_t rtattr_strlcpy(char *dest, const
return ret;
}
-int rtnetlink_send(struct sk_buff *skb, u32 pid, unsigned group, int echo)
+int rtnl_multicast(struct sk_buff *skb, unsigned int group, gfp_t flags)
{
- int err = 0;
+ return nlmsg_multicast(rtnl, skb, 0, group, flags);
+}
- NETLINK_CB(skb).dst_group = group;
- if (echo)
- atomic_inc(&skb->users);
- netlink_broadcast(rtnl, skb, pid, group, GFP_KERNEL);
- if (echo)
- err = netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
- return err;
+int rtnl_unicast(struct sk_buff *skb, u32 pid)
+{
+ return nlmsg_unicast(rtnl, skb, pid);
}
int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics)
@@ -560,9 +557,7 @@ static int rtnl_getlink(struct sk_buff *
goto errout;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(skb).pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
+ err = rtnl_unicast(skb, NETLINK_CB(skb).pid);
errout:
kfree(iw_buf);
dev_put(dev);
@@ -609,8 +604,8 @@ void rtmsg_ifinfo(int type, struct net_d
kfree_skb(skb);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_KERNEL);
+
+ rtnl_multicast(skb, RTNLGRP_LINK, GFP_KERNEL);
}
/* Protected by RTNL sempahore. */
@@ -811,3 +806,5 @@ EXPORT_SYMBOL(rtnl);
EXPORT_SYMBOL(rtnl_lock);
EXPORT_SYMBOL(rtnl_trylock);
EXPORT_SYMBOL(rtnl_unlock);
+EXPORT_SYMBOL(rtnl_multicast);
+EXPORT_SYMBOL(rtnl_unicast);
Index: net-2.6.19.git/net/decnet/dn_table.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_table.c
+++ net-2.6.19.git/net/decnet/dn_table.c
@@ -343,12 +343,8 @@ static void dn_rtmsg_fib(int event, stru
kfree_skb(skb);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_DECnet_ROUTE;
- if (nlh->nlmsg_flags & NLM_F_ECHO)
- atomic_inc(&skb->users);
- netlink_broadcast(rtnl, skb, pid, RTNLGRP_DECnet_ROUTE, GFP_KERNEL);
- if (nlh->nlmsg_flags & NLM_F_ECHO)
- netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
+
+ rtnl_multicast(skb, RTNLGRP_DECnet_ROUTE, GFP_KERNEL);
}
static __inline__ int dn_hash_dump_bucket(struct sk_buff *skb,
Index: net-2.6.19.git/net/ipv4/fib_semantics.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/fib_semantics.c
+++ net-2.6.19.git/net/ipv4/fib_semantics.c
@@ -291,12 +291,8 @@ void rtmsg_fib(int event, u32 key, struc
kfree_skb(skb);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV4_ROUTE;
- if (n->nlmsg_flags&NLM_F_ECHO)
- atomic_inc(&skb->users);
- netlink_broadcast(rtnl, skb, pid, RTNLGRP_IPV4_ROUTE, GFP_KERNEL);
- if (n->nlmsg_flags&NLM_F_ECHO)
- netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
+
+ rtnl_multicast(skb, RTNLGRP_IPV4_ROUTE, GFP_KERNEL);
}
/* Return the first fib alias matching TOS with
Index: net-2.6.19.git/net/sched/act_api.c
===================================================================
--- net-2.6.19.git.orig/net/sched/act_api.c
+++ net-2.6.19.git/net/sched/act_api.c
@@ -459,7 +459,6 @@ static int
act_get_notify(u32 pid, struct nlmsghdr *n, struct tc_action *a, int event)
{
struct sk_buff *skb;
- int err = 0;
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
@@ -468,10 +467,8 @@ act_get_notify(u32 pid, struct nlmsghdr
kfree_skb(skb);
return -EINVAL;
}
- err = netlink_unicast(rtnl, skb, pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
- return err;
+
+ return rtnl_unicast(skb, pid);
}
static struct tc_action *
@@ -592,11 +589,8 @@ static int tca_action_flush(struct rtatt
nlh->nlmsg_flags |= NLM_F_ROOT;
module_put(a->ops->owner);
kfree(a);
- err = rtnetlink_send(skb, pid, RTNLGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
- if (err > 0)
- return 0;
- return err;
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
rtattr_failure:
nlmsg_failure:
@@ -655,11 +649,8 @@ tca_action_gd(struct rtattr *rta, struct
/* now do the delete */
tcf_action_destroy(head, 0);
- ret = rtnetlink_send(skb, pid, RTNLGRP_TC,
- n->nlmsg_flags&NLM_F_ECHO);
- if (ret > 0)
- return 0;
- return ret;
+
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
}
err:
cleanup_a(head);
@@ -674,7 +665,6 @@ static int tcf_add_notify(struct tc_acti
struct sk_buff *skb;
struct rtattr *x;
unsigned char *b;
- int err = 0;
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb)
@@ -697,12 +687,8 @@ static int tcf_add_notify(struct tc_acti
x->rta_len = skb->tail - (u8*)x;
nlh->nlmsg_len = skb->tail - b;
- NETLINK_CB(skb).dst_group = RTNLGRP_TC;
- err = rtnetlink_send(skb, pid, RTNLGRP_TC, flags&NLM_F_ECHO);
- if (err > 0)
- err = 0;
- return err;
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
rtattr_failure:
nlmsg_failure:
Index: net-2.6.19.git/net/sched/cls_api.c
===================================================================
--- net-2.6.19.git.orig/net/sched/cls_api.c
+++ net-2.6.19.git/net/sched/cls_api.c
@@ -366,7 +366,7 @@ static int tfilter_notify(struct sk_buff
return -EINVAL;
}
- return rtnetlink_send(skb, pid, RTNLGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
}
struct tcf_dump_args
Index: net-2.6.19.git/net/sched/sch_api.c
===================================================================
--- net-2.6.19.git.orig/net/sched/sch_api.c
+++ net-2.6.19.git/net/sched/sch_api.c
@@ -815,7 +815,7 @@ static int qdisc_notify(struct sk_buff *
}
if (skb->len)
- return rtnetlink_send(skb, pid, RTNLGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
err_out:
kfree_skb(skb);
@@ -1039,7 +1039,7 @@ static int tclass_notify(struct sk_buff
return -EINVAL;
}
- return rtnetlink_send(skb, pid, RTNLGRP_TC, n->nlmsg_flags&NLM_F_ECHO);
+ return rtnl_multicast(skb, RTNLGRP_TC, GFP_KERNEL);
}
struct qdisc_dump_args
Index: net-2.6.19.git/include/net/genetlink.h
===================================================================
--- net-2.6.19.git.orig/include/net/genetlink.h
+++ net-2.6.19.git/include/net/genetlink.h
@@ -133,11 +133,12 @@ static inline int genlmsg_cancel(struct
* @skb: netlink message as socket buffer
* @pid: own netlink pid to avoid sending to yourself
* @group: multicast group id
+ * @flags: allocation flags
*/
static inline int genlmsg_multicast(struct sk_buff *skb, u32 pid,
- unsigned int group)
+ unsigned int group, gfp_t flags)
{
- return nlmsg_multicast(genl_sock, skb, pid, group);
+ return nlmsg_multicast(genl_sock, skb, pid, group, flags);
}
/**
Index: net-2.6.19.git/include/net/netlink.h
===================================================================
--- net-2.6.19.git.orig/include/net/netlink.h
+++ net-2.6.19.git/include/net/netlink.h
@@ -545,15 +545,16 @@ static inline void nlmsg_free(struct sk_
* @skb: netlink message as socket buffer
* @pid: own netlink pid to avoid sending to yourself
* @group: multicast group id
+ * @flags: allocation flags
*/
static inline int nlmsg_multicast(struct sock *sk, struct sk_buff *skb,
- u32 pid, unsigned int group)
+ u32 pid, unsigned int group, gfp_t flags)
{
int err;
NETLINK_CB(skb).dst_group = group;
- err = netlink_broadcast(sk, skb, pid, group, GFP_KERNEL);
+ err = netlink_broadcast(sk, skb, pid, group, flags);
if (err > 0)
err = 0;
Index: net-2.6.19.git/net/netlabel/netlabel_user.c
===================================================================
--- net-2.6.19.git.orig/net/netlabel/netlabel_user.c
+++ net-2.6.19.git/net/netlabel/netlabel_user.c
@@ -154,5 +154,5 @@ int netlbl_netlink_snd(struct sk_buff *s
*/
int netlbl_netlink_snd_multicast(struct sk_buff *skb, u32 pid, u32 group)
{
- return genlmsg_multicast(skb, pid, group);
+ return genlmsg_multicast(skb, pid, group, GFP_KERNEL);
}
Index: net-2.6.19.git/net/netlink/genetlink.c
===================================================================
--- net-2.6.19.git.orig/net/netlink/genetlink.c
+++ net-2.6.19.git/net/netlink/genetlink.c
@@ -510,7 +510,7 @@ static int genl_ctrl_event(int event, vo
if (IS_ERR(msg))
return PTR_ERR(msg);
- genlmsg_multicast(msg, 0, GENL_ID_CTRL);
+ genlmsg_multicast(msg, 0, GENL_ID_CTRL, GFP_KERNEL);
break;
}
Index: net-2.6.19.git/net/bridge/br_netlink.c
===================================================================
--- net-2.6.19.git.orig/net/bridge/br_netlink.c
+++ net-2.6.19.git/net/bridge/br_netlink.c
@@ -88,8 +88,7 @@ void br_ifinfo_notify(int event, struct
if (err)
goto err_kfree;
- NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_ATOMIC);
+ rtnl_multicast(skb, RTNLGRP_LINK, GFP_ATOMIC);
return;
err_kfree:
Index: net-2.6.19.git/net/core/fib_rules.c
===================================================================
--- net-2.6.19.git.orig/net/core/fib_rules.c
+++ net-2.6.19.git/net/core/fib_rules.c
@@ -354,7 +354,7 @@ static void notify_rule_change(int event
kfree_skb(skb);
netlink_set_err(rtnl, 0, ops->nlgroup, EINVAL);
} else
- netlink_broadcast(rtnl, skb, 0, ops->nlgroup, GFP_KERNEL);
+ rtnl_multicast(skb, ops->nlgroup, GFP_KERNEL);
}
static void attach_rules(struct list_head *rules, struct net_device *dev)
Index: net-2.6.19.git/net/core/neighbour.c
===================================================================
--- net-2.6.19.git.orig/net/core/neighbour.c
+++ net-2.6.19.git/net/core/neighbour.c
@@ -2416,10 +2416,8 @@ void neigh_app_ns(struct neighbour *n)
if (neigh_fill_info(skb, n, 0, 0, RTM_GETNEIGH, NLM_F_REQUEST) <= 0)
kfree_skb(skb);
- else {
- NETLINK_CB(skb).dst_group = RTNLGRP_NEIGH;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_NEIGH, GFP_ATOMIC);
- }
+ else
+ rtnl_multicast(skb, RTNLGRP_NEIGH, GFP_ATOMIC);
}
static void neigh_app_notify(struct neighbour *n)
@@ -2432,10 +2430,8 @@ static void neigh_app_notify(struct neig
if (neigh_fill_info(skb, n, 0, 0, RTM_NEWNEIGH, 0) <= 0)
kfree_skb(skb);
- else {
- NETLINK_CB(skb).dst_group = RTNLGRP_NEIGH;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_NEIGH, GFP_ATOMIC);
- }
+ else
+ rtnl_multicast(skb, RTNLGRP_NEIGH, GFP_ATOMIC);
}
#endif /* CONFIG_ARPD */
Index: net-2.6.19.git/net/core/wireless.c
===================================================================
--- net-2.6.19.git.orig/net/core/wireless.c
+++ net-2.6.19.git/net/core/wireless.c
@@ -1903,8 +1903,8 @@ static inline void rtmsg_iwinfo(struct n
kfree_skb(skb);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_LINK, GFP_ATOMIC);
+
+ rtnl_multicast(skb, RTNLGRP_LINK, GFP_ATOMIC);
}
#endif /* WE_EVENT_RTNETLINK */
Index: net-2.6.19.git/net/decnet/dn_dev.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_dev.c
+++ net-2.6.19.git/net/decnet/dn_dev.c
@@ -757,8 +757,8 @@ static void rtmsg_ifa(int event, struct
netlink_set_err(rtnl, 0, RTNLGRP_DECnet_IFADDR, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_DECnet_IFADDR;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_DECnet_IFADDR, GFP_KERNEL);
+
+ rtnl_multicast(skb, RTNLGRP_DECnet_IFADDR, GFP_KERNEL);
}
static int dn_dev_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
Index: net-2.6.19.git/net/decnet/dn_route.c
===================================================================
--- net-2.6.19.git.orig/net/decnet/dn_route.c
+++ net-2.6.19.git/net/decnet/dn_route.c
@@ -1609,9 +1609,7 @@ int dn_cache_getroute(struct sk_buff *in
goto out_free;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
-
- return err;
+ return rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
out_free:
kfree_skb(skb);
Index: net-2.6.19.git/net/ipv4/devinet.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/devinet.c
+++ net-2.6.19.git/net/ipv4/devinet.c
@@ -1200,7 +1200,7 @@ static void rtmsg_ifa(int event, struct
kfree_skb(skb);
netlink_set_err(rtnl, 0, RTNLGRP_IPV4_IFADDR, EINVAL);
} else
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV4_IFADDR, GFP_KERNEL);
+ rtnl_multicast(skb, RTNLGRP_IPV4_IFADDR, GFP_KERNEL);
}
static struct rtnetlink_link inet_rtnetlink_table[RTM_NR_MSGTYPES] = {
Index: net-2.6.19.git/net/ipv4/ipmr.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/ipmr.c
+++ net-2.6.19.git/net/ipv4/ipmr.c
@@ -312,7 +312,7 @@ static void ipmr_destroy_unres(struct mf
e = NLMSG_DATA(nlh);
e->error = -ETIMEDOUT;
memset(&e->msg, 0, sizeof(e->msg));
- netlink_unicast(rtnl, skb, NETLINK_CB(skb).dst_pid, MSG_DONTWAIT);
+ rtnl_unicast(skb, NETLINK_CB(skb).dst_pid);
} else
kfree_skb(skb);
}
@@ -525,7 +525,7 @@ static void ipmr_cache_resolve(struct mf
e->error = -EMSGSIZE;
memset(&e->msg, 0, sizeof(e->msg));
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(skb).dst_pid, MSG_DONTWAIT);
+ err = rtnl_unicast(skb, NETLINK_CB(skb).dst_pid);
} else
ip_mr_forward(skb, c, 0);
}
Index: net-2.6.19.git/net/ipv4/route.c
===================================================================
--- net-2.6.19.git.orig/net/ipv4/route.c
+++ net-2.6.19.git/net/ipv4/route.c
@@ -2808,9 +2808,7 @@ int inet_rtm_getroute(struct sk_buff *in
goto out_free;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
+ err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
out: return err;
out_free:
Index: net-2.6.19.git/net/ipv6/addrconf.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/addrconf.c
+++ net-2.6.19.git/net/ipv6/addrconf.c
@@ -3268,9 +3268,7 @@ static int inet6_rtm_getaddr(struct sk_b
goto out_free;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
+ err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
out:
in6_ifa_put(ifa);
return err;
@@ -3294,8 +3292,8 @@ static void inet6_ifa_notify(int event,
netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFADDR, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_IFADDR;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFADDR, GFP_ATOMIC);
+
+ rtnl_multicast(skb, RTNLGRP_IPV6_IFADDR, GFP_ATOMIC);
}
static void inline ipv6_store_devconf(struct ipv6_devconf *cnf,
@@ -3448,8 +3446,8 @@ void inet6_ifinfo_notify(int event, stru
netlink_set_err(rtnl, 0, RTNLGRP_IPV6_IFINFO, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_IFINFO;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
+
+ rtnl_multicast(skb, RTNLGRP_IPV6_IFINFO, GFP_ATOMIC);
}
/* Maximum length of prefix_cacheinfo attributes */
@@ -3513,8 +3511,8 @@ static void inet6_prefix_notify(int even
netlink_set_err(rtnl, 0, RTNLGRP_IPV6_PREFIX, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_PREFIX;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_PREFIX, GFP_ATOMIC);
+
+ rtnl_multicast(skb, RTNLGRP_IPV6_PREFIX, GFP_ATOMIC);
}
static struct rtnetlink_link inet6_rtnetlink_table[RTM_NR_MSGTYPES] = {
Index: net-2.6.19.git/net/ipv6/route.c
===================================================================
--- net-2.6.19.git.orig/net/ipv6/route.c
+++ net-2.6.19.git/net/ipv6/route.c
@@ -2161,9 +2161,7 @@ int inet6_rtm_getroute(struct sk_buff *i
goto out_free;
}
- err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
- if (err > 0)
- err = 0;
+ err = rtnl_unicast(skb, NETLINK_CB(in_skb).pid);
out:
return err;
out_free:
@@ -2194,8 +2192,8 @@ void inet6_rt_notify(int event, struct r
netlink_set_err(rtnl, 0, RTNLGRP_IPV6_ROUTE, EINVAL);
return;
}
- NETLINK_CB(skb).dst_group = RTNLGRP_IPV6_ROUTE;
- netlink_broadcast(rtnl, skb, 0, RTNLGRP_IPV6_ROUTE, gfp_any());
+
+ rtnl_multicast(skb, RTNLGRP_IPV6_ROUTE, gfp_any());
}
/*
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications
2006-08-09 20:48 [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications Thomas Graf
` (3 preceding siblings ...)
2006-08-08 22:00 ` [PATCH 4/4] [RTNETLINK]: Unexport global rtnl sock Thomas Graf
@ 2006-08-10 4:16 ` David Miller
4 siblings, 0 replies; 25+ messages in thread
From: David Miller @ 2006-08-10 4:16 UTC (permalink / raw)
To: tgraf; +Cc: netdev
From: Thomas Graf <tgraf@suug.ch>
Date: Wed, 09 Aug 2006 22:48:21 +0200
> Aims at cleaning up rtnetlink event notifications and implements
> real NLM_F_ECHO support.
>
> Please disregard my last IPv4 routing related patchset, I'll
> resubmit it based on this patchset.
All applied, nice work Thomas.
Especially nice to see the RTNL semaphore export go away :-)
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-08 22:00 ` [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb() Thomas Graf
@ 2006-08-10 15:51 ` Alexey Kuznetsov
2006-08-10 19:02 ` Thomas Graf
0 siblings, 1 reply; 25+ messages in thread
From: Alexey Kuznetsov @ 2006-08-10 15:51 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev
Hello!
> This patch handles NLM_F_ECHO in netlink_rcv_skb() to
> handle it in a central point. Most subsystems currently
> interpret NLM_F_ECHO as to just unicast events to the
> originator of the change while the real meaning of the
> flag is to echo the request.
Do not you think it is useless to echo something back to originator,
who just sent it?
Actually, the sense of NLM_F_ECHO was to tell user what happened due to
his request. The answer is not original request, which can contain
some incomplete fields etc., but full information about object
deleted/added/changed. Moreover, the feedback can contain several
messages (though accurately it is done only in net/sched/), f.e. when
the request triggered deletion of one object and addition of another.
Obviously, it cannot be done in a central place.
Normally, it is not needed, "ip route add" does not tell user, what
actually was done, so that it suppresses echo. But for multistage
operation it is absolutely necessary: the answer contains f.e. auto-allocated
handles, which should be given in subsequent requests.
Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications
2006-08-08 22:00 ` [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications Thomas Graf
@ 2006-08-10 18:09 ` Patrick McHardy
2006-08-10 19:04 ` Thomas Graf
0 siblings, 1 reply; 25+ messages in thread
From: Patrick McHardy @ 2006-08-10 18:09 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev
Thomas Graf wrote:
> Setting a socket error on all sockets subscribed to a group
> if an event notificiation of said group fails due to memory
> pressure only confuses applications and is of no use.
>
> This patch removes it all together.
I disagree with this patch, how else are applications supposed
to know when they missed an update and are not in sync anymore?
I actually have a half-finished patch to add this in some spots
where its missing (and uses better error codes).
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-10 15:51 ` Alexey Kuznetsov
@ 2006-08-10 19:02 ` Thomas Graf
2006-08-10 20:32 ` Alexey Kuznetsov
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-10 19:02 UTC (permalink / raw)
To: Alexey Kuznetsov; +Cc: davem, netdev
* Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> 2006-08-10 19:51
> > This patch handles NLM_F_ECHO in netlink_rcv_skb() to
> > handle it in a central point. Most subsystems currently
> > interpret NLM_F_ECHO as to just unicast events to the
> > originator of the change while the real meaning of the
> > flag is to echo the request.
>
> Do not you think it is useless to echo something back to originator,
> who just sent it?
>
> Actually, the sense of NLM_F_ECHO was to tell user what happened due to
> his request. The answer is not original request, which can contain
> some incomplete fields etc., but full information about object
> deleted/added/changed. Moreover, the feedback can contain several
> messages (though accurately it is done only in net/sched/), f.e. when
> the request triggered deletion of one object and addition of another.
>
> Obviously, it cannot be done in a central place.
>
> Normally, it is not needed, "ip route add" does not tell user, what
> actually was done, so that it suppresses echo. But for multistage
> operation it is absolutely necessary: the answer contains f.e. auto-allocated
> handles, which should be given in subsequent requests.
What's wrong with listening to the notification for that purpose?
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications
2006-08-10 18:09 ` Patrick McHardy
@ 2006-08-10 19:04 ` Thomas Graf
2006-08-10 19:08 ` Patrick McHardy
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-10 19:04 UTC (permalink / raw)
To: Patrick McHardy; +Cc: davem, netdev
* Patrick McHardy <kaber@trash.net> 2006-08-10 20:09
> Thomas Graf wrote:
> > Setting a socket error on all sockets subscribed to a group
> > if an event notificiation of said group fails due to memory
> > pressure only confuses applications and is of no use.
> >
> > This patch removes it all together.
>
> I disagree with this patch, how else are applications supposed
> to know when they missed an update and are not in sync anymore?
> I actually have a half-finished patch to add this in some spots
> where its missing (and uses better error codes).
The application has no idea what went wrong nor does it know
for which group so it will have to resync all group subscrptions
and as it only happens due to memory pressure that will fail
anyway.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications
2006-08-10 19:04 ` Thomas Graf
@ 2006-08-10 19:08 ` Patrick McHardy
2006-08-10 19:23 ` Thomas Graf
0 siblings, 1 reply; 25+ messages in thread
From: Patrick McHardy @ 2006-08-10 19:08 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev
Thomas Graf wrote:
> * Patrick McHardy <kaber@trash.net> 2006-08-10 20:09
>
>>I disagree with this patch, how else are applications supposed
>>to know when they missed an update and are not in sync anymore?
>>I actually have a half-finished patch to add this in some spots
>>where its missing (and uses better error codes).
>
>
> The application has no idea what went wrong nor does it know
> for which group so it will have to resync all group subscrptions
> and as it only happens due to memory pressure that will fail
> anyway.
The error code (-ENOMEM) gives it a pretty good idea what went
wrong. Its true that it doesn't know which group was affected
(that could be fixed), but at least it knows that something
went wrong and it needs to resync. If that fails due to memory
shortage as well it can schedule a delayed resync or something,
but without getting notified it has no chance of doing anything
useful. This makes notification essentially useless. If I can't
rely on either getting either a notification or an error, I can't
rely on them at all.
Please put this back in.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications
2006-08-10 19:08 ` Patrick McHardy
@ 2006-08-10 19:23 ` Thomas Graf
2006-08-11 6:02 ` David Miller
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-10 19:23 UTC (permalink / raw)
To: Patrick McHardy; +Cc: davem, netdev
* Patrick McHardy <kaber@trash.net> 2006-08-10 21:08
> The error code (-ENOMEM) gives it a pretty good idea what went
> wrong. Its true that it doesn't know which group was affected
> (that could be fixed), but at least it knows that something
> went wrong and it needs to resync. If that fails due to memory
> shortage as well it can schedule a delayed resync or something,
> but without getting notified it has no chance of doing anything
> useful. This makes notification essentially useless. If I can't
> rely on either getting either a notification or an error, I can't
> rely on them at all.
>
> Please put this back in.
Alright, I think it's pretty much theoretical but it doesn't
really matter to me.
Dave, please revert the whole patchset.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-10 19:02 ` Thomas Graf
@ 2006-08-10 20:32 ` Alexey Kuznetsov
2006-08-10 21:18 ` Thomas Graf
0 siblings, 1 reply; 25+ messages in thread
From: Alexey Kuznetsov @ 2006-08-10 20:32 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev
Hello!
> What's wrong with listening to the notification for that purpose?
Nothing! NLM_F_ECHO _is_ listening for notifications without subscription
to multicast groups and need to figure out what messages are yours.
But beyond this NLM_F_ECHO is totally subset of this.
Which still makes much more sense then echoing of a know thing, does not it?
Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-10 20:32 ` Alexey Kuznetsov
@ 2006-08-10 21:18 ` Thomas Graf
2006-08-11 15:35 ` Alexey Kuznetsov
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-10 21:18 UTC (permalink / raw)
To: Alexey Kuznetsov; +Cc: davem, netdev
Hello
* Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> 2006-08-11 00:32
> Nothing! NLM_F_ECHO _is_ listening for notifications without subscription
> to multicast groups and need to figure out what messages are yours.
> But beyond this NLM_F_ECHO is totally subset of this.
> Which still makes much more sense then echoing of a know thing, does not it?
I get your point and I see the value. Unfortunately, probably due to
lack of documentation, this feature isn't used by any applications I
know of. We even put in the hacks to make identification of own caused
notifications easier by storing the netlink pid of the originator in
the notification message.
I will put this back in (document it! :) and hide it behind
nlmsg_notify() so we do it for all notifications for consistency.
I use echoing of the original request for debuging purposes, it allows
to verify what is actually being parsed at the netlink family specific
parsing function. Using libnl a flag enables NLM_F_ECHO in all messages
and it gets simple to verify what exactly is being seen in the kernel side
parser by looking at the messages log. I agree, there is no functional
value besides the possibility to implement a netlink ping with NLMSG_NOOP.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications
2006-08-10 19:23 ` Thomas Graf
@ 2006-08-11 6:02 ` David Miller
2006-08-11 10:38 ` Thomas Graf
0 siblings, 1 reply; 25+ messages in thread
From: David Miller @ 2006-08-11 6:02 UTC (permalink / raw)
To: tgraf; +Cc: kaber, netdev
From: Thomas Graf <tgraf@suug.ch>
Date: Thu, 10 Aug 2006 21:23:23 +0200
> Dave, please revert the whole patchset.
Done and I've rebased the net-2.6.19 tree as well at:
kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.19.git
Thomas, there was a conflict which I did expect since I added
all the 2.6.18 bug fixes into the tree. The conflict is for
your netlink conversion of the iflink stuff.
I had to fix the -stable tree IFLA_ADDRESS handling because
dev->set_mac_address expects a sockaddr not a raw MAC
address.
I think I resolved the conflict while applying the patch
properly, but if you could take a look I'd appreciate it.
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications
2006-08-11 6:02 ` David Miller
@ 2006-08-11 10:38 ` Thomas Graf
2006-08-11 10:42 ` David Miller
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-11 10:38 UTC (permalink / raw)
To: David Miller; +Cc: kaber, netdev
* David Miller <davem@davemloft.net> 2006-08-10 23:02
> Thomas, there was a conflict which I did expect since I added
> all the 2.6.18 bug fixes into the tree. The conflict is for
> your netlink conversion of the iflink stuff.
>
> I had to fix the -stable tree IFLA_ADDRESS handling because
> dev->set_mac_address expects a sockaddr not a raw MAC
> address.
>
> I think I resolved the conflict while applying the patch
> properly, but if you could take a look I'd appreciate it.
Looks good. You could have used nla_memcpy() instead of
memcpy() but there is no functional difference in this case.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications
2006-08-11 10:38 ` Thomas Graf
@ 2006-08-11 10:42 ` David Miller
0 siblings, 0 replies; 25+ messages in thread
From: David Miller @ 2006-08-11 10:42 UTC (permalink / raw)
To: tgraf; +Cc: kaber, netdev
From: Thomas Graf <tgraf@suug.ch>
Date: Fri, 11 Aug 2006 12:38:22 +0200
> Looks good. You could have used nla_memcpy() instead of
> memcpy() but there is no functional difference in this case.
Right, because the code already verifies that the attribute
length is at least dev->addr_len, and that is the size of
the memcpy we do.
Thanks for checking things out.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-10 21:18 ` Thomas Graf
@ 2006-08-11 15:35 ` Alexey Kuznetsov
2006-08-11 21:47 ` Thomas Graf
2006-08-12 3:05 ` Herbert Xu
0 siblings, 2 replies; 25+ messages in thread
From: Alexey Kuznetsov @ 2006-08-11 15:35 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev
Hello!
> I get your point and I see the value. Unfortunately, probably due to
> lack of documentation, this feature isn't used by any applications I
> know of.
Well, tc was supposed to use it, but this did not happen and
it remained deficient.
> We even put in the hacks to make identification of own caused
> notifications easier by storing the netlink pid of the originator in
> the notification message.
Actually, it was supposed to be done everywhere, but originator info
did not propagate deep enough in many cases, especially in IPv6.
So, this is not a hack, it is a good work. :-)
BTW I have just remembered why it was especially important,
this should be documented as well.
Each socket, which subscribes to multicasts becomes sensitive
to rcvbuf overflows. F.e. when you do control operations on a socket,
which is subscribed to multicasts, the response can be lost in stream
of events and -ENOBUFS generated instead. If it is a daemon, it can resync
the state, but if it is a simple utility, it cannot recover.
Probably, unicasts sent due to NLM_F_ECHO should somehow override
rcvbuf limits.
This reminded me about a capital problem, found by openvz people.
Frankly speaking, I still have no idea how to repair this, probably you
will find a solution.
Look: while a dump, skb allocation can fail (because of many reasons,
the most obvious is that rcvbuf space was eaten by multicasts).
But error is not reported! Oops. The worst thing is that even if an error
is reported, iproute would ignore it.
Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-11 15:35 ` Alexey Kuznetsov
@ 2006-08-11 21:47 ` Thomas Graf
2006-08-12 11:03 ` Alexey Kuznetsov
2006-08-12 3:05 ` Herbert Xu
1 sibling, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-11 21:47 UTC (permalink / raw)
To: Alexey Kuznetsov; +Cc: davem, netdev
* Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> 2006-08-11 19:35
> Well, tc was supposed to use it, but this did not happen and
> it remained deficient.
Makes sense, especially for auto generated handles. I've been listening
to the notifications on a separate socket for this purpose. It would
make sense however to extend a wait_for_ack() function and report
back and eventual echoed objects to have a blocking operation as well.
> Actually, it was supposed to be done everywhere, but originator info
> did not propagate deep enough in many cases, especially in IPv6.
> So, this is not a hack, it is a good work. :-)
It does make sense, the way it has been implemented if at all is
creepy. Even worse, IPv6 is using current->pid, some other code
has been using the pid from NETLINK_CREDS() :-)
> Each socket, which subscribes to multicasts becomes sensitive
> to rcvbuf overflows. F.e. when you do control operations on a socket,
> which is subscribed to multicasts, the response can be lost in stream
> of events and -ENOBUFS generated instead. If it is a daemon, it can resync
> the state, but if it is a simple utility, it cannot recover.
Yes, for that reason it is recommended to use a separate socket
when receiving multicasts. Also because some of the multicast
code is buggy and provides the pid of the requestor's socket to
netlink_broadcast() leading to excluding that socket.
> Probably, unicasts sent due to NLM_F_ECHO should somehow override
> rcvbuf limits.
>
> This reminded me about a capital problem, found by openvz people.
> Frankly speaking, I still have no idea how to repair this, probably you
> will find a solution.
>
> Look: while a dump, skb allocation can fail (because of many reasons,
> the most obvious is that rcvbuf space was eaten by multicasts).
> But error is not reported! Oops. The worst thing is that even if an error
> is reported, iproute would ignore it.
I'm not sure I understand this correctly, if rcvbuf space was eaten
by multicasts subsequent recvmsg() will follow invoking netlink_dump()
again and the dump continues.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-11 15:35 ` Alexey Kuznetsov
2006-08-11 21:47 ` Thomas Graf
@ 2006-08-12 3:05 ` Herbert Xu
2006-08-12 11:05 ` Alexey Kuznetsov
1 sibling, 1 reply; 25+ messages in thread
From: Herbert Xu @ 2006-08-12 3:05 UTC (permalink / raw)
To: Alexey Kuznetsov; +Cc: tgraf, davem, netdev
Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> wrote:
>
> Probably, unicasts sent due to NLM_F_ECHO should somehow override
> rcvbuf limits.
Actually I think the only safe solution is to allocate a separate
socket for multicast messages. In other words, if you want reliable
unicast reception on a socket, don't bind it to a multicast group.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-11 21:47 ` Thomas Graf
@ 2006-08-12 11:03 ` Alexey Kuznetsov
2006-08-12 11:19 ` Thomas Graf
0 siblings, 1 reply; 25+ messages in thread
From: Alexey Kuznetsov @ 2006-08-12 11:03 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev
Hello!
> Makes sense, especially for auto generated handles. I've been listening
> to the notifications on a separate socket for this purpose.
That's... complicated. But cool. :-)
> It does make sense, the way it has been implemented if at all is
> creepy. Even worse, IPv6 is using current->pid, some other code
> has been using the pid from NETLINK_CREDS() :-)
Yeah, some time ago I sent a path replacing those with 0, but
apparently I forgot to grep IPv6. And did not even search for NETLINK_CREDS().
> when receiving multicasts. Also because some of the multicast
> code is buggy and provides the pid of the requestor's socket to
> netlink_broadcast() leading to excluding that socket.
Actually, it was the idea. If requestor asked NLM_F_ECHO and subscribed
to muticasts, it suppresses double notifications. If it did not ask
NLM_F_ECHO, he is not interested in results, he knows what's going on
without this.
F.e. it was used by my implementation in gated: it did not set either
NLM_F_ECHO or even NLM_F_ACK. And when making massive batch updates,
it received nothing back: only errors and updates made by someone else.
> I'm not sure I understand this correctly, if rcvbuf space was eaten
> by multicasts subsequent recvmsg() will follow invoking netlink_dump()
> again and the dump continues.
Indeed. I forgot how it works. :-)
I understood what is happening there. sock_rmalloc() fails not due
to rcvbuf, but because of global resource limiting. Grr... it does not
look solvable. Luckily, in normal kernel this can happen only if 0-order
GFP_KERNEL allocation fails. Not impossible with oom_killer, but it
definitely moves the status of the problem to marginal.
Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-12 3:05 ` Herbert Xu
@ 2006-08-12 11:05 ` Alexey Kuznetsov
0 siblings, 0 replies; 25+ messages in thread
From: Alexey Kuznetsov @ 2006-08-12 11:05 UTC (permalink / raw)
To: Herbert Xu; +Cc: tgraf, davem, netdev
Hello!
> Actually I think the only safe solution is to allocate a separate
> socket for multicast messages. In other words, if you want reliable
> unicast reception on a socket, don't bind it to a multicast group.
Yes, it was the point of my advocacy of NLM_F_ECHO. :-)
Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-12 11:03 ` Alexey Kuznetsov
@ 2006-08-12 11:19 ` Thomas Graf
2006-08-13 13:45 ` Alexey Kuznetsov
0 siblings, 1 reply; 25+ messages in thread
From: Thomas Graf @ 2006-08-12 11:19 UTC (permalink / raw)
To: Alexey Kuznetsov; +Cc: davem, netdev
* Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> 2006-08-12 15:03
> Actually, it was the idea. If requestor asked NLM_F_ECHO and subscribed
> to muticasts, it suppresses double notifications. If it did not ask
> NLM_F_ECHO, he is not interested in results, he knows what's going on
> without this.
>
> F.e. it was used by my implementation in gated: it did not set either
> NLM_F_ECHO or even NLM_F_ACK. And when making massive batch updates,
> it received nothing back: only errors and updates made by someone else.
So we do something like this:
/**
* nlmsg_notify - send a notification netlink message
* @sk: netlink socket to use
* @skb: notification message
* @pid: destination netlink pid for reports or 0
* @group: destination multicast group or 0
* @report: 1 to report back, 0 to disable
* @flags: allocation flags
*/
int nlmsg_notify(struct sock *sk, struct sk_buff *skb, u32 pid,
unsigned int group, int report, gfp_t flags)
{
int err = 0;
if (group) {
int exclude_pid = 0;
if (report) {
atomic_inc(&skb->users);
exclude_pid = pid;
}
/* errors reported via destination sk->sk_err */
nlmsg_multicast(sk, skb, exclude_pid, group, flags);
}
if (report)
err = nlmsg_unicast(sk, skb, pid);
return err;
}
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb()
2006-08-12 11:19 ` Thomas Graf
@ 2006-08-13 13:45 ` Alexey Kuznetsov
0 siblings, 0 replies; 25+ messages in thread
From: Alexey Kuznetsov @ 2006-08-13 13:45 UTC (permalink / raw)
To: Thomas Graf; +Cc: davem, netdev
Hello!
> So we do something like this:
Yes, exactly.
Actually, there was a function with similar functionality: rtnetlink_send().
net/sched/* used it, older net/ipv4/ still did this directly.
Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2006-08-13 13:46 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-09 20:48 [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications Thomas Graf
2006-08-08 22:00 ` [PATCH 1/4] [NETLINK]: Handle NLM_F_ECHO in netlink_rcv_skb() Thomas Graf
2006-08-10 15:51 ` Alexey Kuznetsov
2006-08-10 19:02 ` Thomas Graf
2006-08-10 20:32 ` Alexey Kuznetsov
2006-08-10 21:18 ` Thomas Graf
2006-08-11 15:35 ` Alexey Kuznetsov
2006-08-11 21:47 ` Thomas Graf
2006-08-12 11:03 ` Alexey Kuznetsov
2006-08-12 11:19 ` Thomas Graf
2006-08-13 13:45 ` Alexey Kuznetsov
2006-08-12 3:05 ` Herbert Xu
2006-08-12 11:05 ` Alexey Kuznetsov
2006-08-08 22:00 ` [PATCH 2/4] [RTNETLINK]: Use rtnl_multicast()/rtnl_unicast() Thomas Graf
2006-08-09 21:00 ` [RESEND " Thomas Graf
2006-08-08 22:00 ` [PATCH 3/4] [NETLINK]: Dont set socket error for failed event notifications Thomas Graf
2006-08-10 18:09 ` Patrick McHardy
2006-08-10 19:04 ` Thomas Graf
2006-08-10 19:08 ` Patrick McHardy
2006-08-10 19:23 ` Thomas Graf
2006-08-11 6:02 ` David Miller
2006-08-11 10:38 ` Thomas Graf
2006-08-11 10:42 ` David Miller
2006-08-08 22:00 ` [PATCH 4/4] [RTNETLINK]: Unexport global rtnl sock Thomas Graf
2006-08-10 4:16 ` [PATCHSET]: Clean up netlink NLM_F_ECHO and rtnl event notifications David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).