Wed, Apr 12, 2017 at 02:34:04PM CEST, johan...@sipsolutions.net wrote:
>From: Johannes Berg <johannes.b...@intel.com>
>
>Add the base infrastructure and UAPI for netlink extended ACK
>reporting. All "manual" calls to netlink_ack() pass NULL for
>now and thus don't get extended ACK reporting.
>
>Big thanks goes to Pablo Neira Ayuso for not only bringing up
>the whole topic at netconf (again) but also coming up with the
>nlattr passing trick and various other ideas.
>
>Signed-off-by: Johannes Berg <johannes.b...@intel.com>
>---

[...]


>diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
>index b2c9c26ea30f..7df88770e029 100644
>--- a/include/uapi/linux/netlink.h
>+++ b/include/uapi/linux/netlink.h
>@@ -69,6 +69,10 @@ struct nlmsghdr {
> #define NLM_F_CREATE  0x400   /* Create, if it does not exist */
> #define NLM_F_APPEND  0x800   /* Add to end of list           */
> 
>+/* Flags for ACK message */
>+#define NLM_F_CAPPED  0x100   /* request was capped */

The comment did not help me. What is this supposed to signalize?


>+#define NLM_F_ACK_TLVS        0x200   /* extended ACK TVLs were included */
>+
> /*
>    4.4BSD ADD         NLM_F_CREATE|NLM_F_EXCL
>    4.4BSD CHANGE      NLM_F_REPLACE
>@@ -101,6 +105,33 @@ struct nlmsghdr {

[...]


>diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
>index 85f2fdc360c2..c8bf5136a72b 100644
>--- a/net/decnet/netfilter/dn_rtmsg.c
>+++ b/net/decnet/netfilter/dn_rtmsg.c
>@@ -96,7 +96,7 @@ static unsigned int dnrmg_hook(void *priv,
> }
> 
> 
>-#define RCV_SKB_FAIL(err) do { netlink_ack(skb, nlh, (err)); return; } while 
>(0)
>+#define RCV_SKB_FAIL(err) do { netlink_ack(skb, nlh, (err), NULL); return; } 
>while (0)

You can easily break this into multiple lines to avoid over-80


> 
> static inline void dnrmg_receive_user_skb(struct sk_buff *skb)
> {

[...]


>-void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err)
>+void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err,
>+               const struct netlink_ext_ack *extack)
> {
>       struct sk_buff *skb;
>       struct nlmsghdr *rep;
>       struct nlmsgerr *errmsg;
>       size_t payload = sizeof(*errmsg);
>+      size_t tlvlen = 0;
>       struct netlink_sock *nlk = nlk_sk(NETLINK_CB(in_skb).sk);
>+      unsigned int flags = 0;
> 
>       /* Error messages get the original request appened, unless the user
>-       * requests to cap the error message.
>+       * requests to cap the error message, and get extra error data if
>+       * requested.
>        */
>-      if (!(nlk->flags & NETLINK_F_CAP_ACK) && err)
>-              payload += nlmsg_len(nlh);
>+      if (err) {
>+              if (!(nlk->flags & NETLINK_F_CAP_ACK))
>+                      payload += nlmsg_len(nlh);
>+              else
>+                      flags |= NLM_F_CAPPED;
>+              if (nlk->flags & NETLINK_F_EXT_ACK && extack) {
>+                      if (extack->_msg)
>+                              tlvlen += nla_total_size(strlen(extack->_msg) + 
>1);
>+                      if (extack->bad_attr)
>+                              tlvlen += nla_total_size(sizeof(u32));
>+              }
>+      } else {
>+              flags |= NLM_F_CAPPED;
>+      }
> 
>-      skb = nlmsg_new(payload, GFP_KERNEL);
>+      if (tlvlen)
>+              flags |= NLM_F_ACK_TLVS;

You can move this check and bitset to the end of
        "if (nlk->flags & NETLINK_F_EXT_ACK && extack) { "
as it is directly related to that.


>+
>+      skb = nlmsg_new(payload + tlvlen, GFP_KERNEL);
>       if (!skb) {
>               struct sock *sk;
> 

Reply via email to