Wed, Apr 12, 2017 at 02:34:04PM CEST, johannes@xxxxxxxxxxxxxxxx wrote:
>From: Johannes Berg <johannes.berg@xxxxxxxxx>
>
>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.berg@xxxxxxxxx>
>---
[...]
>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;
>
