Commit ff491a7334acfd74e515c896632e37e401f52676

Authored by Pablo Neira Ayuso
Committed by David S. Miller
1 parent 612e244c12

netlink: change return-value logic of netlink_broadcast()

Currently, netlink_broadcast() reports errors to the caller if no
messages at all were delivered:

1) If, at least, one message has been delivered correctly, returns 0.
2) Otherwise, if no messages at all were delivered due to skb_clone()
   failure, return -ENOBUFS.
3) Otherwise, if there are no listeners, return -ESRCH.

With this patch, the caller knows if the delivery of any of the
messages to the listeners have failed:

1) If it fails to deliver any message (for whatever reason), return
   -ENOBUFS.
2) Otherwise, if all messages were delivered OK, returns 0.
3) Otherwise, if no listeners, return -ESRCH.

In the current ctnetlink code and in Netfilter in general, we can add
reliable logging and connection tracking event delivery by dropping the
packets whose events were not successfully delivered over Netlink. Of
course, this option would be settable via /proc as this approach reduces
performance (in terms of filtered connections per seconds by a stateful
firewall) but providing reliable logging and event delivery (for
conntrackd) in return.

This patch also changes some clients of netlink_broadcast() that
may report ENOBUFS errors via printk. This error handling is not
of any help. Instead, the userspace daemons that are listening to
those netlink messages should resync themselves with the kernel-side
if they hit ENOBUFS.

BTW, netlink_broadcast() clients include those that call
cn_netlink_send(), nlmsg_multicast() and genlmsg_multicast() since they
internally call netlink_broadcast() and return its error value.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

Showing 9 changed files with 28 additions and 48 deletions Side-by-side Diff

drivers/acpi/event.c
... ... @@ -235,11 +235,7 @@
235 235 return result;
236 236 }
237 237  
238   - result =
239   - genlmsg_multicast(skb, 0, acpi_event_mcgrp.id, GFP_ATOMIC);
240   - if (result)
241   - ACPI_DEBUG_PRINT((ACPI_DB_INFO,
242   - "Failed to send a Genetlink message!\n"));
  238 + genlmsg_multicast(skb, 0, acpi_event_mcgrp.id, GFP_ATOMIC);
243 239 return 0;
244 240 }
245 241  
drivers/scsi/scsi_transport_fc.c
... ... @@ -533,12 +533,8 @@
533 533 event->event_code = event_code;
534 534 event->event_data = event_data;
535 535  
536   - err = nlmsg_multicast(scsi_nl_sock, skb, 0, SCSI_NL_GRP_FC_EVENTS,
537   - GFP_KERNEL);
538   - if (err && (err != -ESRCH)) /* filter no recipient errors */
539   - /* nlmsg_multicast already kfree_skb'd */
540   - goto send_fail;
541   -
  536 + nlmsg_multicast(scsi_nl_sock, skb, 0, SCSI_NL_GRP_FC_EVENTS,
  537 + GFP_KERNEL);
542 538 return;
543 539  
544 540 send_fail_skb:
... ... @@ -607,12 +603,8 @@
607 603 event->event_code = FCH_EVT_VENDOR_UNIQUE;
608 604 memcpy(&event->event_data, data_buf, data_len);
609 605  
610   - err = nlmsg_multicast(scsi_nl_sock, skb, 0, SCSI_NL_GRP_FC_EVENTS,
611   - GFP_KERNEL);
612   - if (err && (err != -ESRCH)) /* filter no recipient errors */
613   - /* nlmsg_multicast already kfree_skb'd */
614   - goto send_vendor_fail;
615   -
  606 + nlmsg_multicast(scsi_nl_sock, skb, 0, SCSI_NL_GRP_FC_EVENTS,
  607 + GFP_KERNEL);
616 608 return;
617 609  
618 610 send_vendor_fail_skb:
drivers/scsi/scsi_transport_iscsi.c
... ... @@ -966,15 +966,7 @@
966 966 static int
967 967 iscsi_broadcast_skb(struct sk_buff *skb, gfp_t gfp)
968 968 {
969   - int rc;
970   -
971   - rc = netlink_broadcast(nls, skb, 0, 1, gfp);
972   - if (rc < 0) {
973   - printk(KERN_ERR "iscsi: can not broadcast skb (%d)\n", rc);
974   - return rc;
975   - }
976   -
977   - return 0;
  969 + return netlink_broadcast(nls, skb, 0, 1, gfp);
978 970 }
979 971  
980 972 static int
... ... @@ -1207,7 +1199,7 @@
1207 1199 * the user and when the daemon is restarted it will handle it
1208 1200 */
1209 1201 rc = iscsi_broadcast_skb(skb, GFP_KERNEL);
1210   - if (rc < 0)
  1202 + if (rc == -ESRCH)
1211 1203 iscsi_cls_session_printk(KERN_ERR, session,
1212 1204 "Cannot notify userspace of session "
1213 1205 "event %u. Check iscsi daemon\n",
drivers/video/uvesafb.c
... ... @@ -204,8 +204,11 @@
204 204 } else {
205 205 v86d_started = 1;
206 206 err = cn_netlink_send(m, 0, gfp_any());
  207 + if (err == -ENOBUFS)
  208 + err = 0;
207 209 }
208   - }
  210 + } else if (err == -ENOBUFS)
  211 + err = 0;
209 212  
210 213 if (!err && !(task->t.flags & TF_EXIT))
211 214 err = !wait_for_completion_timeout(task->done,
... ... @@ -1057,10 +1057,7 @@
1057 1057 goto attr_err_out;
1058 1058 genlmsg_end(skb, msg_head);
1059 1059  
1060   - ret = genlmsg_multicast(skb, 0, quota_genl_family.id, GFP_NOFS);
1061   - if (ret < 0 && ret != -ESRCH)
1062   - printk(KERN_ERR
1063   - "VFS: Failed to send notification message: %d\n", ret);
  1060 + genlmsg_multicast(skb, 0, quota_genl_family.id, GFP_NOFS);
1064 1061 return;
1065 1062 attr_err_out:
1066 1063 printk(KERN_ERR "VFS: Not enough space to compose quota message!\n");
lib/kobject_uevent.c
... ... @@ -227,6 +227,9 @@
227 227 NETLINK_CB(skb).dst_group = 1;
228 228 retval = netlink_broadcast(uevent_sock, skb, 0, 1,
229 229 GFP_KERNEL);
  230 + /* ENOBUFS should be handled in userspace */
  231 + if (retval == -ENOBUFS)
  232 + retval = 0;
230 233 } else
231 234 retval = -ENOMEM;
232 235 }
net/netlink/af_netlink.c
... ... @@ -950,6 +950,7 @@
950 950 u32 pid;
951 951 u32 group;
952 952 int failure;
  953 + int delivery_failure;
953 954 int congested;
954 955 int delivered;
955 956 gfp_t allocation;
... ... @@ -999,6 +1000,7 @@
999 1000 p->skb2 = NULL;
1000 1001 } else if ((val = netlink_broadcast_deliver(sk, p->skb2)) < 0) {
1001 1002 netlink_overrun(sk);
  1003 + p->delivery_failure = 1;
1002 1004 } else {
1003 1005 p->congested |= val;
1004 1006 p->delivered = 1;
... ... @@ -1025,6 +1027,7 @@
1025 1027 info.pid = pid;
1026 1028 info.group = group;
1027 1029 info.failure = 0;
  1030 + info.delivery_failure = 0;
1028 1031 info.congested = 0;
1029 1032 info.delivered = 0;
1030 1033 info.allocation = allocation;
1031 1034  
... ... @@ -1045,13 +1048,14 @@
1045 1048 if (info.skb2)
1046 1049 kfree_skb(info.skb2);
1047 1050  
  1051 + if (info.delivery_failure || info.failure)
  1052 + return -ENOBUFS;
  1053 +
1048 1054 if (info.delivered) {
1049 1055 if (info.congested && (allocation & __GFP_WAIT))
1050 1056 yield();
1051 1057 return 0;
1052 1058 }
1053   - if (info.failure)
1054   - return -ENOBUFS;
1055 1059 return -ESRCH;
1056 1060 }
1057 1061 EXPORT_SYMBOL(netlink_broadcast);
... ... @@ -258,7 +258,6 @@
258 258 */
259 259 int wimax_msg_send(struct wimax_dev *wimax_dev, struct sk_buff *skb)
260 260 {
261   - int result;
262 261 struct device *dev = wimax_dev->net_dev->dev.parent;
263 262 void *msg = skb->data;
264 263 size_t size = skb->len;
... ... @@ -266,11 +265,9 @@
266 265  
267 266 d_printf(1, dev, "CTX: wimax msg, %zu bytes\n", size);
268 267 d_dump(2, dev, msg, size);
269   - result = genlmsg_multicast(skb, 0, wimax_gnl_mcg.id, GFP_KERNEL);
270   - d_printf(1, dev, "CTX: genl multicast result %d\n", result);
271   - if (result == -ESRCH) /* Nobody connected, ignore it */
272   - result = 0; /* btw, the skb is freed already */
273   - return result;
  268 + genlmsg_multicast(skb, 0, wimax_gnl_mcg.id, GFP_KERNEL);
  269 + d_printf(1, dev, "CTX: genl multicast done\n");
  270 + return 0;
274 271 }
275 272 EXPORT_SYMBOL_GPL(wimax_msg_send);
276 273  
... ... @@ -163,16 +163,12 @@
163 163 struct device *dev = wimax_dev_to_dev(wimax_dev);
164 164 d_fnstart(3, dev, "(wimax_dev %p report_skb %p)\n",
165 165 wimax_dev, report_skb);
166   - if (report_skb == NULL)
  166 + if (report_skb == NULL) {
  167 + result = -ENOMEM;
167 168 goto out;
168   - genlmsg_end(report_skb, header);
169   - result = genlmsg_multicast(report_skb, 0, wimax_gnl_mcg.id, GFP_KERNEL);
170   - if (result == -ESRCH) /* Nobody connected, ignore it */
171   - result = 0; /* btw, the skb is freed already */
172   - if (result < 0) {
173   - dev_err(dev, "RE_STCH: Error sending: %d\n", result);
174   - nlmsg_free(report_skb);
175 169 }
  170 + genlmsg_end(report_skb, header);
  171 + genlmsg_multicast(report_skb, 0, wimax_gnl_mcg.id, GFP_KERNEL);
176 172 out:
177 173 d_fnend(3, dev, "(wimax_dev %p report_skb %p) = %d\n",
178 174 wimax_dev, report_skb, result);