Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 17/20] net/fib_rules: Update fib_nl_dumprule for strict data checking
From: David Ahern @ 2018-10-04 21:33 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-1-dsahern@kernel.org>

From: David Ahern <dsahern@gmail.com>

Update fib_nl_dumprule for strict data checking. If the flag is set,
the dump request is expected to have fib_rule_hdr struct as the header.
All elements of the struct are expected to be 0 and no attributes can
be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/fib_rules.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 0ff3953f64aa..e3cf50728d0a 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -1063,13 +1063,47 @@ static int dump_rules(struct sk_buff *skb, struct netlink_callback *cb,
 	return err;
 }
 
+static int fib_valid_dumprule(const struct nlmsghdr *nlh,
+			      struct netlink_ext_ack *extack)
+{
+	struct fib_rule_hdr *frh;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*frh))) {
+		NL_SET_ERR_MSG(extack, "Invalid header");
+		return -EINVAL;
+	}
+
+	frh = nlmsg_data(nlh);
+	if (frh->dst_len || frh->src_len || frh->tos || frh->table ||
+	    frh->res1 || frh->res2 || frh->action || frh->flags) {
+		NL_SET_ERR_MSG(extack,
+			       "Invalid values in header for dump request");
+		return -EINVAL;
+	}
+
+	if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*frh))) {
+		NL_SET_ERR_MSG(extack, "Invalid data after header");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int fib_nl_dumprule(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct fib_rules_ops *ops;
 	int idx = 0, family;
 
-	family = rtnl_msg_family(cb->nlh);
+	if (cb->strict_check) {
+		int err = fib_valid_dumprule(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
+	family = rtnl_msg_family(nlh);
 	if (family != AF_UNSPEC) {
 		/* Protocol specific dump request */
 		ops = lookup_rules_ops(net, family);
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 14/20] net/neighbor: Update neigh_dump_info for strict data checking
From: David Ahern @ 2018-10-04 21:33 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-1-dsahern@kernel.org>

From: David Ahern <dsahern@gmail.com>

Update neigh_dump_info for strict data checking. If the flag is set,
the dump request is expected to have an ndmsg struct as the header
potentially followed by one or more attributes. Any data passed in the
header or as an attribute is taken as a request to influence the data
returned. Only values supported by the dump handler are allowed to be
non-0 or set in the request. At the moment only the NDA_IFINDEX and
NDA_MASTER attributes are supported.

Existing code does not fail the dump if nlmsg_parse fails. That behavior
is kept for non-strict checking.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/neighbour.c | 59 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index b06f794bf91e..3130d010b7ad 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2428,13 +2428,14 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 
 static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	struct netlink_ext_ack *extack = cb->extack;
 	const struct nlmsghdr *nlh = cb->nlh;
 	struct neigh_dump_filter filter = {};
 	struct nlattr *tb[NDA_MAX + 1];
 	struct neigh_table *tbl;
 	int t, family, s_t;
 	int proxy = 0;
-	int err;
+	int err, i;
 
 	family = ((struct rtgenmsg *)nlmsg_data(nlh))->rtgen_family;
 
@@ -2445,20 +2446,56 @@ static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 	    ((struct ndmsg *)nlmsg_data(nlh))->ndm_flags == NTF_PROXY)
 		proxy = 1;
 
-	err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL,
-			  cb->extack);
-	if (!err) {
-		if (tb[NDA_IFINDEX]) {
-			if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
-				return -EINVAL;
-			filter.dev_idx = nla_get_u32(tb[NDA_IFINDEX]);
+	if (cb->strict_check) {
+		struct ndmsg *ndm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ndm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		ndm = nlmsg_data(nlh);
+		if (ndm->ndm_pad1  || ndm->ndm_pad2  || ndm->ndm_ifindex ||
+		    ndm->ndm_state || ndm->ndm_flags || ndm->ndm_type) {
+			NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+			return -EINVAL;
 		}
-		if (tb[NDA_MASTER]) {
-			if (nla_len(tb[NDA_MASTER]) != sizeof(u32))
+	}
+
+	err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL, extack);
+	if (err < 0) {
+		if (cb->strict_check)
+			return -EINVAL;
+		goto walk_entries;
+	}
+
+	for (i = 0; i <= NDA_MAX; ++i) {
+		if (!tb[i])
+			continue;
+		switch (i) {
+		case NDA_IFINDEX:
+			if (nla_len(tb[i]) != sizeof(u32)) {
+				NL_SET_ERR_MSG(extack, "Invalid IFINDEX attribute");
 				return -EINVAL;
-			filter.master_idx = nla_get_u32(tb[NDA_MASTER]);
+			}
+			filter.dev_idx = nla_get_u32(tb[i]);
+			break;
+		case NDA_MASTER:
+			if (nla_len(tb[i]) != sizeof(u32)) {
+				NL_SET_ERR_MSG(extack, "Invalid MASTER attribute");
+				return -EINVAL;
+			}
+			filter.master_idx = nla_get_u32(tb[i]);
+			break;
+		default:
+			if (cb->strict_check) {
+				NL_SET_ERR_MSG(extack, "Unsupported attribute in dump request");
+				return -EINVAL;
+			}
 		}
 	}
+
+walk_entries:
 	s_t = cb->args[0];
 
 	for (t = 0; t < NEIGH_NR_TABLES; t++) {
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 13/20] rtnetlink: Update fib dumps for strict data checking
From: David Ahern @ 2018-10-04 21:33 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-1-dsahern@kernel.org>

From: David Ahern <dsahern@gmail.com>

Add helper to check netlink message for route dumps. If the strict flag
is set the dump request is expected to have an rtmsg struct as the header.
All elements of the struct are expected to be 0 with the exception of
rtm_flags (which is used by both ipv4 and ipv6 dumps) and no attributes
can be appended. rtm_flags can only have RTM_F_CLONED and RTM_F_PREFIX
set.

Update inet_dump_fib, inet6_dump_fib, mpls_dump_routes, ipmr_rtm_dumproute,
and ip6mr_rtm_dumproute to call this helper if strict data checking is
enabled.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/ip_fib.h    |  2 ++
 net/ipv4/fib_frontend.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 net/ipv4/ipmr.c         |  9 +++++++++
 net/ipv6/ip6_fib.c      |  8 ++++++++
 net/ipv6/ip6mr.c        |  9 +++++++++
 net/mpls/af_mpls.c      |  8 ++++++++
 6 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index f7c109e37298..9846b79c9ee1 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -452,4 +452,6 @@ static inline void fib_proc_exit(struct net *net)
 
 u32 ip_mtu_from_fib_result(struct fib_result *res, __be32 daddr);
 
+int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
+			  struct netlink_ext_ack *extack);
 #endif  /* _NET_FIB_H */
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 30e2bcc3ef2a..1583ec0a5154 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -802,8 +802,41 @@ static int inet_rtm_newroute(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return err;
 }
 
+int ip_valid_fib_dump_req(const struct nlmsghdr *nlh,
+			  struct netlink_ext_ack *extack)
+{
+	struct rtmsg *rtm;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*rtm))) {
+		NL_SET_ERR_MSG(extack, "Invalid header");
+		return -EINVAL;
+	}
+
+	rtm = nlmsg_data(nlh);
+	if (rtm->rtm_dst_len || rtm->rtm_src_len  || rtm->rtm_tos   ||
+	    rtm->rtm_table   || rtm->rtm_protocol || rtm->rtm_scope ||
+	    rtm->rtm_type) {
+		NL_SET_ERR_MSG(extack,
+			       "Invalid values in header for dump request");
+		return -EINVAL;
+	}
+
+	if (rtm->rtm_flags & ~(RTM_F_CLONED | RTM_F_PREFIX)) {
+		NL_SET_ERR_MSG(extack, "Invalid flags for dump request");
+		return -EINVAL;
+	}
+	if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*rtm))) {
+		NL_SET_ERR_MSG(extack, "Invalid data after header");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ip_valid_fib_dump_req);
+
 static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	unsigned int h, s_h;
 	unsigned int e = 0, s_e;
@@ -811,8 +844,14 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 	struct hlist_head *head;
 	int dumped = 0, err;
 
-	if (nlmsg_len(cb->nlh) >= sizeof(struct rtmsg) &&
-	    ((struct rtmsg *) nlmsg_data(cb->nlh))->rtm_flags & RTM_F_CLONED)
+	if (cb->strict_check) {
+		err = ip_valid_fib_dump_req(nlh, cb->extack);
+		if (err)
+			return err;
+	}
+
+	if (nlmsg_len(nlh) >= sizeof(struct rtmsg) &&
+	    ((struct rtmsg *)nlmsg_data(nlh))->rtm_flags & RTM_F_CLONED)
 		return skb->len;
 
 	s_h = cb->args[0];
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index e6c48e08d53d..2a7963beecfb 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -2527,6 +2527,15 @@ static int ipmr_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 
 static int ipmr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
+
+	if (cb->strict_check) {
+		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	return mr_rtm_dumproute(skb, cb, ipmr_mr_table_iter,
 				_ipmr_fill_mroute, &mfc_unres_lock);
 }
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 5516f55e214b..123786684476 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -568,6 +568,7 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
 
 static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	unsigned int h, s_h;
 	unsigned int e = 0, s_e;
@@ -577,6 +578,13 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 	struct hlist_head *head;
 	int res = 0;
 
+	if (cb->strict_check) {
+		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	s_h = cb->args[0];
 	s_e = cb->args[1];
 
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 6f07b8380425..8a94500c5532 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -2457,6 +2457,15 @@ static void mrt6msg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt)
 
 static int ip6mr_rtm_dumproute(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
+
+	if (cb->strict_check) {
+		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	return mr_rtm_dumproute(skb, cb, ip6mr_mr_table_iter,
 				_ip6mr_fill_mroute, &mfc_unres_lock);
 }
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 55a30ee3d820..3e33934751b4 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -2017,6 +2017,7 @@ static int mpls_dump_route(struct sk_buff *skb, u32 portid, u32 seq, int event,
 
 static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct mpls_route __rcu **platform_label;
 	size_t platform_labels;
@@ -2024,6 +2025,13 @@ static int mpls_dump_routes(struct sk_buff *skb, struct netlink_callback *cb)
 
 	ASSERT_RTNL();
 
+	if (cb->strict_check) {
+		int err = ip_valid_fib_dump_req(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	index = cb->args[0];
 	if (index < MPLS_LABEL_FIRST_UNRESERVED)
 		index = MPLS_LABEL_FIRST_UNRESERVED;
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 15/20] net/neighbor: Update neightbl_dump_info for strict data checking
From: David Ahern @ 2018-10-04 21:33 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-1-dsahern@kernel.org>

From: David Ahern <dsahern@gmail.com>

Update neightbl_dump_info for strict data checking. If the flag is set,
the dump request is expected to have an ndtmsg struct as the header.
All elements of the struct are expected to be 0 and no attributes can
be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/neighbour.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 3130d010b7ad..8e07b92403ab 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2164,15 +2164,47 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh,
 	return err;
 }
 
+static int neightbl_valid_dump_info(const struct nlmsghdr *nlh,
+				    struct netlink_ext_ack *extack)
+{
+	struct ndtmsg *ndtm;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ndtm))) {
+		NL_SET_ERR_MSG(extack, "Invalid header");
+		return -EINVAL;
+	}
+
+	ndtm = nlmsg_data(nlh);
+	if (ndtm->ndtm_pad1  || ndtm->ndtm_pad2) {
+		NL_SET_ERR_MSG(extack, "Invalid values in header for dump request");
+		return -EINVAL;
+	}
+
+	if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ndtm))) {
+		NL_SET_ERR_MSG(extack, "Invalid data after header");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	int family, tidx, nidx = 0;
 	int tbl_skip = cb->args[0];
 	int neigh_skip = cb->args[1];
 	struct neigh_table *tbl;
 
-	family = ((struct rtgenmsg *) nlmsg_data(cb->nlh))->rtgen_family;
+	if (cb->strict_check) {
+		int err = neightbl_valid_dump_info(nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
+	family = ((struct rtgenmsg *)nlmsg_data(nlh))->rtgen_family;
 
 	for (tidx = 0; tidx < NEIGH_NR_TABLES; tidx++) {
 		struct neigh_parms *p;
@@ -2185,7 +2217,7 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 			continue;
 
 		if (neightbl_fill_info(skb, tbl, NETLINK_CB(cb->skb).portid,
-				       cb->nlh->nlmsg_seq, RTM_NEWNEIGHTBL,
+				       nlh->nlmsg_seq, RTM_NEWNEIGHTBL,
 				       NLM_F_MULTI) < 0)
 			break;
 
@@ -2200,7 +2232,7 @@ static int neightbl_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
 
 			if (neightbl_fill_param_info(skb, tbl, p,
 						     NETLINK_CB(cb->skb).portid,
-						     cb->nlh->nlmsg_seq,
+						     nlh->nlmsg_seq,
 						     RTM_NEWNEIGHTBL,
 						     NLM_F_MULTI) < 0)
 				goto out;
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 16/20] net/namespace: Update rtnl_net_dumpid for strict data checking
From: David Ahern @ 2018-10-04 21:33 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-1-dsahern@kernel.org>

From: David Ahern <dsahern@gmail.com>

Update rtnl_net_dumpid for strict data checking. If the flag is set,
the dump request is expected to have an rtgenmsg struct as the header
which has the family as the only element. No data may be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/net_namespace.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 670c84b1bfc2..63659c512ba8 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -844,6 +844,7 @@ static int rtnl_net_dumpid_one(int id, void *peer, void *data)
 
 static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct rtnl_net_dump_cb net_cb = {
 		.net = net,
@@ -853,6 +854,13 @@ static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
 		.s_idx = cb->args[0],
 	};
 
+	if (cb->strict_check) {
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(struct rtgenmsg))) {
+			NL_SET_ERR_MSG(cb->extack, "Unknown data in dump request");
+			return -EINVAL;
+		}
+	}
+
 	spin_lock_bh(&net->nsid_lock);
 	idr_for_each(&net->netns_ids, rtnl_net_dumpid_one, &net_cb);
 	spin_unlock_bh(&net->nsid_lock);
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 18/20] net/ipv6: Update ip6addrlbl_dump for strict data checking
From: David Ahern @ 2018-10-04 21:33 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-1-dsahern@kernel.org>

From: David Ahern <dsahern@gmail.com>

Update ip6addrlbl_dump for strict data checking. If the flag is set,
the dump request is expected to have an ifaddrlblmsg struct as the
header. All elements of the struct are expected to be 0 and no
attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv6/addrlabel.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/addrlabel.c b/net/ipv6/addrlabel.c
index 1d6ced37ad71..10556049cc44 100644
--- a/net/ipv6/addrlabel.c
+++ b/net/ipv6/addrlabel.c
@@ -458,20 +458,53 @@ static int ip6addrlbl_fill(struct sk_buff *skb,
 	return 0;
 }
 
+static int ip6addrlbl_valid_dump_req(const struct nlmsghdr *nlh,
+				     struct netlink_ext_ack *extack)
+{
+	struct ifaddrlblmsg *ifal;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ifal))) {
+		NL_SET_ERR_MSG(extack, "Invalid header");
+		return -EINVAL;
+	}
+
+	ifal = nlmsg_data(nlh);
+	if (ifal->__ifal_reserved || ifal->ifal_prefixlen ||
+	    ifal->ifal_flags || ifal->ifal_index || ifal->ifal_seq) {
+		NL_SET_ERR_MSG(extack,
+			       "Invalid values in header for dump request");
+		return -EINVAL;
+	}
+
+	if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ifal))) {
+		NL_SET_ERR_MSG(extack, "Invalid data after header");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int ip6addrlbl_dump(struct sk_buff *skb, struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct ip6addrlbl_entry *p;
 	int idx = 0, s_idx = cb->args[0];
 	int err;
 
+	if (cb->strict_check) {
+		err = ip6addrlbl_valid_dump_req(nlh, cb->extack);
+		if (err)
+			return err;
+	}
+
 	rcu_read_lock();
 	hlist_for_each_entry_rcu(p, &net->ipv6.ip6addrlbl_table.head, list) {
 		if (idx >= s_idx) {
 			err = ip6addrlbl_fill(skb, p,
 					      net->ipv6.ip6addrlbl_table.seq,
 					      NETLINK_CB(cb->skb).portid,
-					      cb->nlh->nlmsg_seq,
+					      nlh->nlmsg_seq,
 					      RTM_NEWADDRLABEL,
 					      NLM_F_MULTI);
 			if (err < 0)
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 20/20] net/bridge: Update br_mdb_dump for strict data checking
From: David Ahern @ 2018-10-04 21:33 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-1-dsahern@kernel.org>

From: David Ahern <dsahern@gmail.com>

Update br_mdb_dump for strict data checking. If the flag is set,
the dump request is expected to have a br_port_msg struct as the
header. All elements of the struct are expected to be 0 and no
attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/bridge/br_mdb.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index a4a848bf827b..7beeee658d6c 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -162,6 +162,28 @@ static int br_mdb_fill_info(struct sk_buff *skb, struct netlink_callback *cb,
 	return err;
 }
 
+static int br_mdb_valid_dump_req(const struct nlmsghdr *nlh,
+				 struct netlink_ext_ack *extack)
+{
+	struct br_port_msg *bpm;
+
+	if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*bpm))) {
+		NL_SET_ERR_MSG(extack, "Invalid header");
+		return -EINVAL;
+	}
+	if (bpm->ifindex) {
+		NL_SET_ERR_MSG(extack,
+			       "Filtering by device index is not supported");
+		return -EINVAL;
+	}
+	if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*bpm))) {
+		NL_SET_ERR_MSG(extack, "Invalid data after header");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int br_mdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	struct net_device *dev;
@@ -169,6 +191,13 @@ static int br_mdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	struct nlmsghdr *nlh = NULL;
 	int idx = 0, s_idx;
 
+	if (cb->strict_check) {
+		int err = br_mdb_valid_dump_req(cb->nlh, cb->extack);
+
+		if (err)
+			return err;
+	}
+
 	s_idx = cb->args[0];
 
 	rcu_read_lock();
-- 
2.11.0

^ permalink raw reply related

* [PATCH net-next 19/20] net: Update netconf dump handlers for strict data checking
From: David Ahern @ 2018-10-04 21:33 UTC (permalink / raw)
  To: netdev, davem; +Cc: christian, jbenc, stephen, David Ahern
In-Reply-To: <20181004213355.14899-1-dsahern@kernel.org>

From: David Ahern <dsahern@gmail.com>

Update inet_netconf_dump_devconf, inet6_netconf_dump_devconf, and
mpls_netconf_dump_devconf for strict data checking. If the flag is set,
the dump request is expected to have an netconfmsg struct as the header.
The struct only has the family member and no attributes can be appended.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/devinet.c  | 22 +++++++++++++++++++---
 net/ipv6/addrconf.c | 22 +++++++++++++++++++---
 net/mpls/af_mpls.c  | 18 +++++++++++++++++-
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index af968d4fe4fc..595706d6b672 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2069,6 +2069,7 @@ static int inet_netconf_get_devconf(struct sk_buff *in_skb,
 static int inet_netconf_dump_devconf(struct sk_buff *skb,
 				     struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	int h, s_h;
 	int idx, s_idx;
@@ -2076,6 +2077,21 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
 	struct in_device *in_dev;
 	struct hlist_head *head;
 
+	if (cb->strict_check) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct netconfmsg *ncm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
 
@@ -2095,7 +2111,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
 			if (inet_netconf_fill_devconf(skb, dev->ifindex,
 						      &in_dev->cnf,
 						      NETLINK_CB(cb->skb).portid,
-						      cb->nlh->nlmsg_seq,
+						      nlh->nlmsg_seq,
 						      RTM_NEWNETCONF,
 						      NLM_F_MULTI,
 						      NETCONFA_ALL) < 0) {
@@ -2112,7 +2128,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
 		if (inet_netconf_fill_devconf(skb, NETCONFA_IFINDEX_ALL,
 					      net->ipv4.devconf_all,
 					      NETLINK_CB(cb->skb).portid,
-					      cb->nlh->nlmsg_seq,
+					      nlh->nlmsg_seq,
 					      RTM_NEWNETCONF, NLM_F_MULTI,
 					      NETCONFA_ALL) < 0)
 			goto done;
@@ -2123,7 +2139,7 @@ static int inet_netconf_dump_devconf(struct sk_buff *skb,
 		if (inet_netconf_fill_devconf(skb, NETCONFA_IFINDEX_DEFAULT,
 					      net->ipv4.devconf_dflt,
 					      NETLINK_CB(cb->skb).portid,
-					      cb->nlh->nlmsg_seq,
+					      nlh->nlmsg_seq,
 					      RTM_NEWNETCONF, NLM_F_MULTI,
 					      NETCONFA_ALL) < 0)
 			goto done;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 693199a29426..9dfe6c2106c1 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -666,6 +666,7 @@ static int inet6_netconf_get_devconf(struct sk_buff *in_skb,
 static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 				      struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	int h, s_h;
 	int idx, s_idx;
@@ -673,6 +674,21 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 	struct inet6_dev *idev;
 	struct hlist_head *head;
 
+	if (cb->strict_check) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct netconfmsg *ncm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
 
@@ -692,7 +708,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 			if (inet6_netconf_fill_devconf(skb, dev->ifindex,
 						       &idev->cnf,
 						       NETLINK_CB(cb->skb).portid,
-						       cb->nlh->nlmsg_seq,
+						       nlh->nlmsg_seq,
 						       RTM_NEWNETCONF,
 						       NLM_F_MULTI,
 						       NETCONFA_ALL) < 0) {
@@ -709,7 +725,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 		if (inet6_netconf_fill_devconf(skb, NETCONFA_IFINDEX_ALL,
 					       net->ipv6.devconf_all,
 					       NETLINK_CB(cb->skb).portid,
-					       cb->nlh->nlmsg_seq,
+					       nlh->nlmsg_seq,
 					       RTM_NEWNETCONF, NLM_F_MULTI,
 					       NETCONFA_ALL) < 0)
 			goto done;
@@ -720,7 +736,7 @@ static int inet6_netconf_dump_devconf(struct sk_buff *skb,
 		if (inet6_netconf_fill_devconf(skb, NETCONFA_IFINDEX_DEFAULT,
 					       net->ipv6.devconf_dflt,
 					       NETLINK_CB(cb->skb).portid,
-					       cb->nlh->nlmsg_seq,
+					       nlh->nlmsg_seq,
 					       RTM_NEWNETCONF, NLM_F_MULTI,
 					       NETCONFA_ALL) < 0)
 			goto done;
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index 3e33934751b4..b80b00b55bdf 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -1263,6 +1263,7 @@ static int mpls_netconf_get_devconf(struct sk_buff *in_skb,
 static int mpls_netconf_dump_devconf(struct sk_buff *skb,
 				     struct netlink_callback *cb)
 {
+	const struct nlmsghdr *nlh = cb->nlh;
 	struct net *net = sock_net(skb->sk);
 	struct hlist_head *head;
 	struct net_device *dev;
@@ -1270,6 +1271,21 @@ static int mpls_netconf_dump_devconf(struct sk_buff *skb,
 	int idx, s_idx;
 	int h, s_h;
 
+	if (cb->strict_check) {
+		struct netlink_ext_ack *extack = cb->extack;
+		struct netconfmsg *ncm;
+
+		if (nlh->nlmsg_len < nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid header");
+			return -EINVAL;
+		}
+
+		if (nlh->nlmsg_len != nlmsg_msg_size(sizeof(*ncm))) {
+			NL_SET_ERR_MSG(extack, "Invalid data after header");
+			return -EINVAL;
+		}
+	}
+
 	s_h = cb->args[0];
 	s_idx = idx = cb->args[1];
 
@@ -1286,7 +1302,7 @@ static int mpls_netconf_dump_devconf(struct sk_buff *skb,
 				goto cont;
 			if (mpls_netconf_fill_devconf(skb, mdev,
 						      NETLINK_CB(cb->skb).portid,
-						      cb->nlh->nlmsg_seq,
+						      nlh->nlmsg_seq,
 						      RTM_NEWNETCONF,
 						      NLM_F_MULTI,
 						      NETCONFA_ALL) < 0) {
-- 
2.11.0

^ permalink raw reply related

* [PATCH iproute2-next] libnetlink: Use NLMSG_LENGTH to set nlmsg_len
From: David Ahern @ 2018-10-04 21:37 UTC (permalink / raw)
  To: netdev; +Cc: stephen, David Ahern

From: David Ahern <dsahern@gmail.com>

Some of the inner headers are not 4-byte aligned, so use
NLMSG_LENGTH instead of sizeof(req) to set nlmsg_len.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 lib/libnetlink.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index 9276be29363f..449536197f60 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -238,7 +238,7 @@ int rtnl_addrdump_req(struct rtnl_handle *rth, int family)
 		struct nlmsghdr nlh;
 		struct ifaddrmsg ifm;
 	} req = {
-		.nlh.nlmsg_len = sizeof(req),
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrmsg)),
 		.nlh.nlmsg_type = RTM_GETADDR,
 		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
 		.nlh.nlmsg_seq = rth->dump = ++rth->seq,
@@ -254,7 +254,7 @@ int rtnl_addrlbldump_req(struct rtnl_handle *rth, int family)
 		struct nlmsghdr nlh;
 		struct ifaddrlblmsg ifal;
 	} req = {
-		.nlh.nlmsg_len = sizeof(req),
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrlblmsg)),
 		.nlh.nlmsg_type = RTM_GETADDRLABEL,
 		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
 		.nlh.nlmsg_seq = rth->dump = ++rth->seq,
@@ -296,7 +296,7 @@ int rtnl_ruledump_req(struct rtnl_handle *rth, int family)
 		struct nlmsghdr nlh;
 		struct fib_rule_hdr frh;
 	} req = {
-		.nlh.nlmsg_len = sizeof(req),
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct fib_rule_hdr)),
 		.nlh.nlmsg_type = RTM_GETRULE,
 		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
 		.nlh.nlmsg_seq = rth->dump = ++rth->seq,
@@ -312,7 +312,7 @@ int rtnl_neighdump_req(struct rtnl_handle *rth, int family)
 		struct nlmsghdr nlh;
 		struct ndmsg ndm;
 	} req = {
-		.nlh.nlmsg_len = sizeof(req),
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ndmsg)),
 		.nlh.nlmsg_type = RTM_GETNEIGH,
 		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
 		.nlh.nlmsg_seq = rth->dump = ++rth->seq,
@@ -328,7 +328,7 @@ int rtnl_neightbldump_req(struct rtnl_handle *rth, int family)
 		struct nlmsghdr nlh;
 		struct ndtmsg ndtmsg;
 	} req = {
-		.nlh.nlmsg_len = sizeof(req),
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ndtmsg)),
 		.nlh.nlmsg_type = RTM_GETNEIGHTBL,
 		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
 		.nlh.nlmsg_seq = rth->dump = ++rth->seq,
@@ -344,7 +344,7 @@ int rtnl_mdbdump_req(struct rtnl_handle *rth, int family)
 		struct nlmsghdr nlh;
 		struct br_port_msg bpm;
 	} req = {
-		.nlh.nlmsg_len = sizeof(req),
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct br_port_msg)),
 		.nlh.nlmsg_type = RTM_GETMDB,
 		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
 		.nlh.nlmsg_seq = rth->dump = ++rth->seq,
@@ -360,7 +360,7 @@ int rtnl_netconfdump_req(struct rtnl_handle *rth, int family)
 		struct nlmsghdr nlh;
 		struct netconfmsg ncm;
 	} req = {
-		.nlh.nlmsg_len = sizeof(req),
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct netconfmsg)),
 		.nlh.nlmsg_type = RTM_GETNETCONF,
 		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
 		.nlh.nlmsg_seq = rth->dump = ++rth->seq,
@@ -376,7 +376,7 @@ int rtnl_nsiddump_req(struct rtnl_handle *rth, int family)
 		struct nlmsghdr nlh;
 		struct rtgenmsg rtm;
 	} req = {
-		.nlh.nlmsg_len = sizeof(req),
+		.nlh.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg)),
 		.nlh.nlmsg_type = RTM_GETNSID,
 		.nlh.nlmsg_flags = NLM_F_DUMP | NLM_F_REQUEST,
 		.nlh.nlmsg_seq = rth->dump = ++rth->seq,
-- 
2.11.0

^ permalink raw reply related

* Your images 22
From: Joanna @ 2018-10-04 11:24 UTC (permalink / raw)
  To: netdev

Have photos for cutting out or retouching?

We are one image team and we do editing for your the e-commerce photos,
industry photos or portrait photo.

If you need test editing then let me know.
Waiting for your reply and the photo work.

Thanks,
Joanna

^ permalink raw reply

* Your images 22
From: Joanna @ 2018-10-04 14:53 UTC (permalink / raw)
  To: netdev

Have photos for cutting out or retouching?

We are one image team and we do editing for your the e-commerce photos,
industry photos or portrait photo.

If you need test editing then let me know.
Waiting for your reply and the photo work.

Thanks,
Joanna

^ permalink raw reply

* Re: [PATCH bpf-next 1/6] bpf: introduce BPF_PROG_TYPE_FILE_FILTER
From: Al Viro @ 2018-10-05  4:46 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S . Miller, daniel, luto, netdev, linux-kernel, kernel-team
In-Reply-To: <20181004025750.498303-2-ast@kernel.org>

On Wed, Oct 03, 2018 at 07:57:45PM -0700, Alexei Starovoitov wrote:
 
> @@ -15,6 +15,7 @@
>  #include <linux/bpf.h>
>  #include <linux/bpf-cgroup.h>
>  #include <net/sock.h>
> +#include <../fs/mount.h>

No.

> +	struct file *file = NULL;
> +	struct inode *inode;
> +	struct super_block *sb;
> +	struct mount *mnt;

Fuck, no.

> +	case offsetof(struct bpf_file_info, mnt_id):
> +		/* dst = real_mount(file->f_path.mnt)->mnt_id */
> +		mnt = real_mount(LD_1(file->f_path.mnt));
> +		LD_n(mnt->mnt_id);

NAK.  Anything in struct mount is private to just a couple of
files in fs/*.c.  Don't do that.  And keep in mind that internal
details can and will be changed at zero notice, so be careful
with adding such stuff.

Another problem is your direct poking in ->i_ino.  It's not
something directly exposed to userland at the moment and it should
not become such.  Filesystem has every right to have ->getattr()
set ->ino (== ->st_ino value) in whichever way it likes; the same
goes for ->dev.

^ permalink raw reply

* Re: [PATCH net] net: dsa: b53: Keep CPU port as tagged in all VLANs
From: David Miller @ 2018-10-05  4:53 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, lists, andrew, vivien.didelot, linux-kernel
In-Reply-To: <20181005032413.8866-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu,  4 Oct 2018 20:24:13 -0700

> Commit c499696e7901 ("net: dsa: b53: Stop using dev->cpu_port
> incorrectly") was a bit too trigger happy in removing the CPU port from
> the VLAN membership because we rely on DSA to program the CPU port VLAN,
> which it does, except it does not bother itself with tagged/untagged and
> just usese untagged.
> 
> Having the CPU port "follow" the user ports tagged/untagged is not great
> and does not allow for properly differentiating, so keep the CPU port
> tagged in all VLANs.
> 
> Reported-by: Gerhard Wiesinger <lists@wiesinger.com>
> Fixes: c499696e7901 ("net: dsa: b53: Stop using dev->cpu_port incorrectly")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH RFC,net-next 0/3] ip_tunnel: specify tunnel type via template
From: Pablo Neira Ayuso @ 2018-10-04 21:58 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, netfilter-devel, roopa, amir, pshelar, u9012063
In-Reply-To: <20181004121357.3efa7275@cakuba.netronome.com>

Hi Jakub,

On Thu, Oct 04, 2018 at 12:13:57PM -0700, Jakub Kicinski wrote:
> On Thu,  4 Oct 2018 02:03:42 +0200, Pablo Neira Ayuso wrote:
> > Hi,
> > 
> > The following patchset adds a new field to the tunnel metadata template
> > to restrict the configuration to a given tunnel driver. Currently, a
> > misconfiguration may result in packets going to the wrong tunnel driver.
> > 
> > Although we have the tunnel option flags, they are not mandatory for
> > some tunnel drivers, eg. vxlan, which may use it or not; and gre which
> > does not use them.
> 
> Option flags are necessary because interpretation of option blob is
> entirely protocol-specific.
> 
> > This patch updates tc's tunnel action and netfilter's tunnel extension
> > to use this new field. OVS netlink interface has been left unset, although they
> > could be updated to use this.
> > 
> > By extending the existing tc action to support the IP_TUNNEL_INFO_BRIDGE
> > mode, I think it should be possible to expose IP_TUNNEL_TYPE_VLAN too,
> > although this patchset doesn't address this scenario.
> > 
> > The field is initialized to zero, which maps to IP_TUNNEL_TYPE_UNSPEC to
> > retain the existing behaviour, so the existing flexibility is still in
> > place while this new feature is added.
> > 
> > Cc'ing people that git annotate show as dealing with these bits more
> > recently.
> 
> What practical scenario are you trying to address here?

Incorrect configuration. The tunnel template defines an ID field, this
ID means vni in vxlan, but it also means session in erspan. If a
packet that should go to vxlan tunnel device (to be encapsulated using
vni 5) ends up in a gre/erspan device, you will get an erspan packet
with session 5. With this new tunnel type field, you can restrict
the tunnel template to work _only_ for a given tunnel device driver.
Hence, if the packet ends up in the wrong tunnel device driver due to
incorrect configuration, packet gets dropped.

> Have you seen
> https://www.mail-archive.com/netdev@vger.kernel.org/msg250705.html
> ?

Hm, I remember to have seen some hw offload driver code that is making
assumptions on the destination ports to pick the tunnel protocol,
based on what the act_key_tunnel is passing.

This patchset may probably help there too since act_key_tunnel will
convey the tunnel type, given this can now be made explicit. The tc
action parsing from the driver can annotate the tunnel type that has
been set in this rule via act_tunnel_key, then validate that follow up
mirred action points to a tunnel device of the same type.

Thanks.

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
From: Alexei Starovoitov @ 2018-10-04 22:10 UTC (permalink / raw)
  To: Jesper Dangaard Brouer; +Cc: Daniel Borkmann, ast, netdev, Jiri Olsa, acme
In-Reply-To: <20181004222231.2edd5add@redhat.com>

On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote:
> 
> > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:  
> > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> [...]
> > >>
> > >> If the purpose of the patch is to give user space visibility into
> > >> bpf prog load/unload as a notification, then I completely agree that
> > >> some notification mechanism is necessary.  
> > 
> > Yeah, I did only regard it as only that, nothing more. Some means
> > of timeline and notification that can be kept in a record in user
> > space and later retrieved e.g. for introspection on what has been
> > loaded.
> > 
> > >> I've started working on such mechanism via perf ring buffer which is
> > >> the fastest mechanism we have in the kernel so far.
> > >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/  
> > 
> > That one is definitely needed in any case to resolve the kallsyms
> > limitations, and it does have overlap in that in either case we
> > want to look at past BPF programs that have been unloaded in the
> > meantime, so I don't have a strong preference either way, and the
> > former is needed in any case. Though thought was that audit might
> > be an option for those not running profiling daemons 24/7, but
> > presumably bpftool could be extended to record these events as
> > well if we don't want to reuse audit infra.
> 
> Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> these events.  I do acknowledge that this perf event is relevant,
> especially for catching the kernel symbols (I need that myself), but it
> does not cover my use-case.
> 
> My use-case is to 24/7 collect and keep records in userspace, and have a
> timeline of these notifications, for later retrieval.  The idea is that
> our support engineers can look at these records when troubleshooting
> the system.  And the plan is also to collect these records as part of
> our sosreport tool, which is part of the support case.

I don't think you're implying that prog load/unload should be spamming dmesg
and auditd not even running...
Also auditd has to be changed to support retrieving prog related info (like license)
via sys_bpf system call when it sees prog_id.
Since it has to change it can just as easily use perf ring buffer
to receive notifications.
So we solve notification problem once and all user space tools can use it.

^ permalink raw reply

* Re: [PATCH v2] typo fix in Documentation/networking/af_xdp.rst
From: Daniel Borkmann @ 2018-10-04 22:14 UTC (permalink / raw)
  To: Björn Töpel, kdjimeli; +Cc: Netdev
In-Reply-To: <CAJ+HfNiSn5ehkArKMgO3P-xwnDedVJvOTQqHw0-st9y8PuoH_w@mail.gmail.com>

On 10/04/2018 07:38 PM, Björn Töpel wrote:
> Den tors 4 okt. 2018 kl 19:03 skrev Konrad Djimeli <kdjimeli@igalia.com>:
>>
>> Fix a simple typo: Completetion -> Completion
>>
>> Signed-off-by: Konrad Djimeli <kdjimeli@igalia.com>
>> ---
>> Changes in v2:
>> - Update line below to be same length as text above
>>
>>  Documentation/networking/af_xdp.rst | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/networking/af_xdp.rst b/Documentation/networking/af_xdp.rst
>> index ff929cfab4f4..4ae4f9d8f8fe 100644
>> --- a/Documentation/networking/af_xdp.rst
>> +++ b/Documentation/networking/af_xdp.rst
>> @@ -159,8 +159,8 @@ log2(2048) LSB of the addr will be masked off, meaning that 2048, 2050
>>  and 3000 refers to the same chunk.
>>
>>
>> -UMEM Completetion Ring
>> -~~~~~~~~~~~~~~~~~~~~~~
>> +UMEM Completion Ring
>> +~~~~~~~~~~~~~~~~~~~~
>>
>>  The Completion Ring is used transfer ownership of UMEM frames from
>>  kernel-space to user-space. Just like the Fill ring, UMEM indicies are
>> --
>> 2.17.1
>>
> 
> Thanks Konrad! For future patches, you should tag your patch with
> 'bpf' or 'bpf-next' as pointed out in
> Documentation/bpf/bpf_devel_QA.rst. I guess this should go to 'bpf'.
> 
> Acked-by: Björn Töpel <bjorn.topel@intel.com>

I think it's so minor that bpf-next is totally fine, therefore applied
there, thanks Konrad!

^ permalink raw reply

* Re: [PATCH 2/2] can: tcan4x5x: Add tcan4x5x driver to the kernel
From: Wolfgang Grandegger @ 2018-10-05  5:56 UTC (permalink / raw)
  To: Dan Murphy, mkl, davem; +Cc: linux-can, netdev, linux-kernel, Mario.Huettel
In-Reply-To: <7a75a096-d07f-693e-74b6-74a76c00acf5@ti.com>

Hello Dan,

Am 04.10.2018 um 22:26 schrieb Dan Murphy:
> Wolfgang
> 
> On 09/26/2018 12:54 PM, Wolfgang Grandegger wrote:
>> Hello,
>>
>> I wonder why you do not extend the existing MCAN driver by implementing
>> an interface to access the hardware. Would that be feasible?
>>
> 
> I have created a m_can_core code base that can be used by other hardware that
> have special needs.
> 
> So I have created the m_can_core, m_can and the tcan4x5x drivers.

Great, I still think it's a good idea to have just one "m_can" driver.

> I can RFC the code to see if this is what is expected.
> It is not 100% working but it is close enough for a directional call.

That would be nice! Most of the SPI accesses are pure register accesses.
A few read/write more bytes at a time (for data, etc.) but that could be
handled by appropriate interface functions. One general problem is that
SPI accesses are not possible from interrupt context requiring threads
or work queues. Also NAPI is usually not used.

Other opinions?

Wolfgang.

PS: I have added Mario to the CC. Maybe he could test a common driver on
his M_CAN hardware.

^ permalink raw reply

* [PATCH iproute2 net-next v2 0/6] Introduce the taprio scheduler
From: Vinicius Costa Gomes @ 2018-10-04 23:17 UTC (permalink / raw)
  To: netdev
  Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri,
	jesus.sanchez-palencia, ilias.apalodimas, simon.fok

Hi,

Changes from v1:
  - Remove references to the "H" (Set-Gates-And-Hold-MAC) and "R"
    (Set-Gates-And-Release-MAC) commands, as these commands will only
    be used when Frame Preemption support is added (David Ahern);
  - Moved the functions that print and read commands to be closer (David
    Ahern);

Changes from RFC:
  - Removed support for the sched-file parameter, in favor of
    supporting the batch mode feature;

This is the iproute2 side of the taprio v1 series.

Please see the kernel side cover letter for more information about how
to test this.

Cheers,

^ permalink raw reply

* [PATCH iproute2 net-next v2 6/6] taprio: Add manpage for tc-taprio(8)
From: Vinicius Costa Gomes @ 2018-10-04 23:17 UTC (permalink / raw)
  To: netdev
  Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri,
	jesus.sanchez-palencia, ilias.apalodimas, simon.fok
In-Reply-To: <20181004231711.6058-1-vinicius.gomes@intel.com>

This documents the parameters and provides an example of usage.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 man/man8/tc-taprio.8 | 142 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 man/man8/tc-taprio.8

diff --git a/man/man8/tc-taprio.8 b/man/man8/tc-taprio.8
new file mode 100644
index 00000000..92055b43
--- /dev/null
+++ b/man/man8/tc-taprio.8
@@ -0,0 +1,142 @@
+.TH TAPRIO 8 "25 Sept 2018" "iproute2" "Linux"
+.SH NAME
+TAPRIO \- Time Aware Priority Shaper
+.SH SYNOPSIS
+.B tc qdisc ... dev
+dev
+.B parent
+classid
+.B [ handle
+major:
+.B ] taprio num_tc
+tcs
+.ti +8
+.B map
+P0 P1 P2 ...
+.B queues
+count1@offset1 count2@offset2 ...
+.ti +8
+.B base-time
+base-time
+.B clockid
+clockid
+.ti +8
+.B sched-entry
+<command 1> <gate mask 1> <interval 1>
+.ti +8
+.B sched-entry
+<command 2> <gate mask 2> <interval 2>
+.ti +8
+.B sched-entry
+<command 3> <gate mask 3> <interval 3>
+.ti +8
+.B sched-entry
+<command N> <gate mask N> <interval N>
+
+.SH DESCRIPTION
+The TAPRIO qdisc implements a simplified version of the scheduling
+state machine defined by IEEE 802.1Q-2018 Section 8.6.9, which allows
+configuration of a sequence of gate states, where each gate state
+allows outgoing traffic for a subset (potentially empty) of traffic
+classes.
+
+How traffic is mapped to different hardware queues is similar to
+.BR mqprio(8)
+and so the
+.B map
+and
+.Q queues
+parameters have the same meaning.
+
+The other parameters specify the schedule, and at what point in time
+it should start (it can behave as the schedule started in the past).
+
+.SH PARAMETERS
+.TP
+num_tc
+.BR
+Number of traffic classes to use. Up to 16 classes supported.
+
+.TP
+map
+.br
+The priority to traffic class map. Maps priorities 0..15 to a specified
+traffic class. See
+.BR mqprio(8)
+for more details.
+
+.TP
+queues
+.br
+Provide count and offset of queue range for each traffic class. In the
+format,
+.B count@offset.
+Queue ranges for each traffic classes cannot overlap and must be a
+contiguous range of queues.
+
+.TP
+base-time
+.br
+Specifies the instant in nanoseconds, using the reference of
+.B clockid,
+defining the time when the schedule starts. If 'base-time' is a time
+in the past, the schedule will start at
+
+base-time + (N * cycle-time)
+
+where N is the smallest integer so the resulting time is greater than
+"now", and "cycle-time" is the sum of all the intervals of the entries
+in the schedule;
+
+.TP
+clockid
+.br
+Specifies the clock to be used by qdisc's internal timer for measuring
+time and scheduling events.
+
+.TP
+sched-entry
+.br
+There may multiple
+.B sched-entry
+parameters in a single schedule. Each one has the
+
+sched-entry <command> <gatemask> <interval>
+
+format. The only supported <command> is "S", which
+means "SetGateStates", following the IEEE 802.1Q-2018 definition
+(Table 8-7). <gate mask> is a bitmask where each bit is a associated
+with a traffic class, so bit 0 (the least significant bit) being "on"
+means that traffic class 0 is "active" for that schedule entry.
+<interval> is a time duration, in nanoseconds, that specifies for how
+long that state defined by <command> and <gate mask> should be held
+before moving to the next entry.
+
+.SH EXAMPLES
+
+The following example shows how an traffic schedule with three traffic
+classes ("num_tc 3"), which are separated different traffic classes,
+we are going to call these TC 0, TC 1 and TC 2. We could read the
+"map" parameter below as: traffic with priority 3 is classified as TC
+0, priority 2 is classified as TC 1 and the rest is classified as TC
+2.
+
+The schedule will start at instant 1528743495910289987 using the
+reference CLOCK_TAI. The schedule is composed of three entries each of
+300us duration.
+
+.EX
+# tc qdisc replace dev eth0 parent root handle 100 taprio \\
+              num_tc 3 \\
+              map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \\
+              queues 1@0 1@1 2@2 \\
+              base-time 1528743495910289987 \\
+              sched-entry S 01 300000 \\
+              sched-entry S 02 300000 \\
+              sched-entry S 04 300000 \\
+              clockid CLOCK_TAI
+.EE
+
+
+.SH AUTHORS
+Vinicius Costa Gomes <vinicius.gomes@intel.com>
-- 
2.19.0

^ permalink raw reply related

* [PATCH iproute2 net-next v2 1/6] utils: Implement get_s64()
From: Vinicius Costa Gomes @ 2018-10-04 23:17 UTC (permalink / raw)
  To: netdev
  Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri,
	jesus.sanchez-palencia, ilias.apalodimas, simon.fok
In-Reply-To: <20181004231711.6058-1-vinicius.gomes@intel.com>

Add this helper to read signed 64-bit integers from a string.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 include/utils.h |  1 +
 lib/utils.c     | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/utils.h b/include/utils.h
index 8cb4349e..58574a05 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -139,6 +139,7 @@ int get_time_rtt(unsigned *val, const char *arg, int *raw);
 #define get_byte get_u8
 #define get_ushort get_u16
 #define get_short get_s16
+int get_s64(__s64 *val, const char *arg, int base);
 int get_u64(__u64 *val, const char *arg, int base);
 int get_u32(__u32 *val, const char *arg, int base);
 int get_s32(__s32 *val, const char *arg, int base);
diff --git a/lib/utils.c b/lib/utils.c
index e87ecf31..1b84b801 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -383,6 +383,27 @@ int get_u8(__u8 *val, const char *arg, int base)
 	return 0;
 }
 
+int get_s64(__s64 *val, const char *arg, int base)
+{
+	long res;
+	char *ptr;
+
+	errno = 0;
+
+	if (!arg || !*arg)
+		return -1;
+	res = strtoll(arg, &ptr, base);
+	if (!ptr || ptr == arg || *ptr)
+		return -1;
+	if ((res == LLONG_MIN || res == LLONG_MAX) && errno == ERANGE)
+		return -1;
+	if (res > INT64_MAX || res < INT64_MIN)
+		return -1;
+
+	*val = res;
+	return 0;
+}
+
 int get_s32(__s32 *val, const char *arg, int base)
 {
 	long res;
-- 
2.19.0

^ permalink raw reply related

* [PATCH iproute2 net-next v2 4/6] include: add definitions for taprio [DO NOT COMMIT]
From: Vinicius Costa Gomes @ 2018-10-04 23:17 UTC (permalink / raw)
  To: netdev
  Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri,
	jesus.sanchez-palencia, ilias.apalodimas, simon.fok
In-Reply-To: <20181004231711.6058-1-vinicius.gomes@intel.com>

DO NOT COMMIT

This patch exists only to ease the testing, until this header is
updated with the definitions from the kernel.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
 include/uapi/linux/pkt_sched.h | 52 ++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 8975fd1a..89ee47c2 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -395,9 +395,9 @@ enum {
 struct tc_htb_xstats {
 	__u32 lends;
 	__u32 borrows;
-	__u32 giants;	/* too big packets (rate will not be accurate) */
-	__u32 tokens;
-	__u32 ctokens;
+	__u32 giants;	/* unused since 'Make HTB scheduler work with TSO.' */
+	__s32 tokens;
+	__s32 ctokens;
 };
 
 /* HFSC section */
@@ -1084,4 +1084,50 @@ enum {
 	CAKE_ATM_MAX
 };
 
+
+/* TAPRIO */
+enum {
+	TC_TAPRIO_CMD_SET_GATES = 0x00,
+	TC_TAPRIO_CMD_SET_AND_HOLD = 0x01,
+	TC_TAPRIO_CMD_SET_AND_RELEASE = 0x02,
+};
+
+enum {
+	TCA_TAPRIO_SCHED_ENTRY_UNSPEC,
+	TCA_TAPRIO_SCHED_ENTRY_INDEX, /* u32 */
+	TCA_TAPRIO_SCHED_ENTRY_CMD, /* u8 */
+	TCA_TAPRIO_SCHED_ENTRY_GATE_MASK, /* u32 */
+	TCA_TAPRIO_SCHED_ENTRY_INTERVAL, /* u32 */
+	__TCA_TAPRIO_SCHED_ENTRY_MAX,
+};
+#define TCA_TAPRIO_SCHED_ENTRY_MAX (__TCA_TAPRIO_SCHED_ENTRY_MAX - 1)
+
+/* The format for schedule entry list is:
+ * [TCA_TAPRIO_SCHED_ENTRY_LIST]
+ *   [TCA_TAPRIO_SCHED_ENTRY]
+ *     [TCA_TAPRIO_SCHED_ENTRY_CMD]
+ *     [TCA_TAPRIO_SCHED_ENTRY_GATES]
+ *     [TCA_TAPRIO_SCHED_ENTRY_INTERVAL]
+ */
+enum {
+	TCA_TAPRIO_SCHED_UNSPEC,
+	TCA_TAPRIO_SCHED_ENTRY,
+	__TCA_TAPRIO_SCHED_MAX,
+};
+
+#define TCA_TAPRIO_SCHED_MAX (__TCA_TAPRIO_SCHED_MAX - 1)
+
+enum {
+	TCA_TAPRIO_ATTR_UNSPEC,
+	TCA_TAPRIO_ATTR_PRIOMAP, /* struct tc_mqprio_qopt */
+	TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST, /* nested of entry */
+	TCA_TAPRIO_ATTR_SCHED_BASE_TIME, /* s64 */
+	TCA_TAPRIO_ATTR_SCHED_SINGLE_ENTRY, /* single entry */
+	TCA_TAPRIO_ATTR_SCHED_CLOCKID, /* s32 */
+	TCA_TAPRIO_PAD,
+	__TCA_TAPRIO_ATTR_MAX,
+};
+
+#define TCA_TAPRIO_ATTR_MAX (__TCA_TAPRIO_ATTR_MAX - 1)
+
 #endif
-- 
2.19.0

^ permalink raw reply related

* [PATCH iproute2 net-next v2 2/6] include: Add helper to retrieve a __s64 from a netlink msg
From: Vinicius Costa Gomes @ 2018-10-04 23:17 UTC (permalink / raw)
  To: netdev
  Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri,
	jesus.sanchez-palencia, ilias.apalodimas, simon.fok
In-Reply-To: <20181004231711.6058-1-vinicius.gomes@intel.com>

This allows signed 64-bit integers to be retrieved from a netlink
message.
---
 include/libnetlink.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index 9d9249e6..88164975 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -185,6 +185,13 @@ static inline __u64 rta_getattr_u64(const struct rtattr *rta)
 	memcpy(&tmp, RTA_DATA(rta), sizeof(__u64));
 	return tmp;
 }
+static inline __s64 rta_getattr_s64(const struct rtattr *rta)
+{
+	__s64 tmp;
+
+	memcpy(&tmp, RTA_DATA(rta), sizeof(__s64));
+	return tmp;
+}
 static inline const char *rta_getattr_str(const struct rtattr *rta)
 {
 	return (const char *)RTA_DATA(rta);
-- 
2.19.0

^ permalink raw reply related

* [PATCH iproute2 net-next v2 3/6] libnetlink: Add helper for getting a __s32 from netlink msgs
From: Vinicius Costa Gomes @ 2018-10-04 23:17 UTC (permalink / raw)
  To: netdev
  Cc: Jesus Sanchez-Palencia, jhs, xiyou.wangcong, jiri,
	ilias.apalodimas, simon.fok
In-Reply-To: <20181004231711.6058-1-vinicius.gomes@intel.com>

From: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>

This function retrieves a signed 32-bit integer from a netlink message
and returns it.

Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
---
 include/libnetlink.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/libnetlink.h b/include/libnetlink.h
index 88164975..79ba793e 100644
--- a/include/libnetlink.h
+++ b/include/libnetlink.h
@@ -185,6 +185,10 @@ static inline __u64 rta_getattr_u64(const struct rtattr *rta)
 	memcpy(&tmp, RTA_DATA(rta), sizeof(__u64));
 	return tmp;
 }
+static inline __s32 rta_getattr_s32(const struct rtattr *rta)
+{
+	return *(__s32 *)RTA_DATA(rta);
+}
 static inline __s64 rta_getattr_s64(const struct rtattr *rta)
 {
 	__s64 tmp;
-- 
2.19.0

^ permalink raw reply related

* [PATCH iproute2 net-next v2 5/6] tc: Add support for configuring the taprio scheduler
From: Vinicius Costa Gomes @ 2018-10-04 23:17 UTC (permalink / raw)
  To: netdev
  Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri,
	jesus.sanchez-palencia, ilias.apalodimas, simon.fok
In-Reply-To: <20181004231711.6058-1-vinicius.gomes@intel.com>

This traffic scheduler allows traffic classes states (transmission
allowed/not allowed, in the simplest case) to be scheduled, according
to a pre-generated time sequence. This is the basis of the IEEE
802.1Qbv specification.

Example configuration:

tc qdisc replace dev enp3s0 parent root handle 100 taprio \
          num_tc 3 \
	  map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
	  queues 1@0 1@1 2@2 \
	  base-time 1528743495910289987 \
	  sched-entry S 01 300000 \
	  sched-entry S 02 300000 \
	  sched-entry S 04 300000 \
	  clockid CLOCK_TAI

The configuration format is similar to mqprio. The main difference is
the presence of a schedule, built by multiple "sched-entry"
definitions, each entry has the following format:

     sched-entry <CMD> <GATE MASK> <INTERVAL>

The only supported <CMD> is "S", which means "SetGateStates",
following the IEEE 802.1Qbv-2015 definition (Table 8-6). <GATE MASK>
is a bitmask where each bit is a associated with a traffic class, so
bit 0 (the least significant bit) being "on" means that traffic class
0 is "active" for that schedule entry. <INTERVAL> is a time duration
in nanoseconds that specifies for how long that state defined by <CMD>
and <GATE MASK> should be held before moving to the next entry.

This schedule is circular, that is, after the last entry is executed
it starts from the first one, indefinitely.

The other parameters can be defined as follows:

 - base-time: specifies the instant when the schedule starts, if
  'base-time' is a time in the past, the schedule will start at

 	      base-time + (N * cycle-time)

   where N is the smallest integer so the resulting time is greater
   than "now", and "cycle-time" is the sum of all the intervals of the
   entries in the schedule;

 - clockid: specifies the reference clock to be used;

The parameters should be similar to what the IEEE 802.1Q family of
specification defines.

Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
---
 tc/Makefile   |   1 +
 tc/q_taprio.c | 400 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 401 insertions(+)
 create mode 100644 tc/q_taprio.c

diff --git a/tc/Makefile b/tc/Makefile
index 5a1a7ff9..25a28284 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -74,6 +74,7 @@ TCMODULES += e_bpf.o
 TCMODULES += f_matchall.o
 TCMODULES += q_cbs.o
 TCMODULES += q_etf.o
+TCMODULES += q_taprio.o
 
 TCSO :=
 ifeq ($(TC_CONFIG_ATM),y)
diff --git a/tc/q_taprio.c b/tc/q_taprio.c
new file mode 100644
index 00000000..562dacb8
--- /dev/null
+++ b/tc/q_taprio.c
@@ -0,0 +1,400 @@
+/*
+ * q_taprio.c	Time Aware Priority Scheduler
+ *
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ * Authors:	Vinicius Costa Gomes <vinicius.gomes@intel.com>
+ * 		Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+
+#include "utils.h"
+#include "tc_util.h"
+#include "list.h"
+
+struct sched_entry {
+	struct list_head list;
+	uint32_t index;
+	uint32_t interval;
+	uint32_t gatemask;
+	uint8_t cmd;
+};
+
+#define CLOCKID_INVALID (-1)
+static const struct static_clockid {
+	const char *name;
+	clockid_t clockid;
+} clockids_sysv[] = {
+	{ "REALTIME", CLOCK_REALTIME },
+	{ "TAI", CLOCK_TAI },
+	{ "BOOTTIME", CLOCK_BOOTTIME },
+	{ "MONOTONIC", CLOCK_MONOTONIC },
+	{ NULL }
+};
+
+static void explain(void)
+{
+	fprintf(stderr, "Usage: ... taprio clockid CLOCKID\n");
+	fprintf(stderr, "                  [num_tc NUMBER] [map P0 P1 ...] ");
+	fprintf(stderr, "                  [queues COUNT@OFFSET COUNT@OFFSET COUNT@OFFSET ...] ");
+	fprintf(stderr, "                  [ [sched-entry index cmd gate-mask interval] ... ] ");
+	fprintf(stderr, "                  [base-time time] ");
+	fprintf(stderr, "\nCLOCKID must be a valid SYS-V id (i.e. CLOCK_TAI)");
+	fprintf(stderr, "\n");
+}
+
+static void explain_clockid(const char *val)
+{
+	fprintf(stderr, "taprio: illegal value for \"clockid\": \"%s\".\n", val);
+	fprintf(stderr, "It must be a valid SYS-V id (i.e. CLOCK_TAI)\n");
+}
+
+static int get_clockid(__s32 *val, const char *arg)
+{
+	const struct static_clockid *c;
+
+	/* Drop the CLOCK_ prefix if that is being used. */
+	if (strcasestr(arg, "CLOCK_") != NULL)
+		arg += sizeof("CLOCK_") - 1;
+
+	for (c = clockids_sysv; c->name; c++) {
+		if (strcasecmp(c->name, arg) == 0) {
+			*val = c->clockid;
+
+			return 0;
+		}
+	}
+
+	return -1;
+}
+
+static const char* get_clock_name(clockid_t clockid)
+{
+	const struct static_clockid *c;
+
+	for (c = clockids_sysv; c->name; c++) {
+		if (clockid == c->clockid)
+			return c->name;
+	}
+
+	return "invalid";
+}
+
+static const char *entry_cmd_to_str(__u8 cmd)
+{
+	switch (cmd) {
+	case TC_TAPRIO_CMD_SET_GATES:
+		return "S";
+	default:
+		return "Invalid";
+	}
+}
+
+static int str_to_entry_cmd(const char *str)
+{
+	if (strcmp(str, "S") == 0)
+		return TC_TAPRIO_CMD_SET_GATES;
+
+	return -1;
+}
+
+static int add_sched_list(struct list_head *sched_entries, struct nlmsghdr *n)
+{
+	struct sched_entry *e;
+
+	list_for_each_entry(e, sched_entries, list) {
+		struct rtattr *a;
+
+		a = addattr_nest(n, 1024, TCA_TAPRIO_SCHED_ENTRY);
+
+		addattr_l(n, 1024, TCA_TAPRIO_SCHED_ENTRY_CMD, &e->cmd, sizeof(e->cmd));
+		addattr_l(n, 1024, TCA_TAPRIO_SCHED_ENTRY_GATE_MASK, &e->gatemask, sizeof(e->gatemask));
+		addattr_l(n, 1024, TCA_TAPRIO_SCHED_ENTRY_INTERVAL, &e->interval, sizeof(e->interval));
+
+		addattr_nest_end(n, a);
+	}
+
+	return 0;
+}
+
+static void explain_sched_entry(void)
+{
+	fprintf(stderr, "Usage: ... taprio ... sched-entry <cmd> <gate mask> <interval>\n");
+}
+
+static struct sched_entry *create_entry(uint32_t gatemask, uint32_t interval, uint8_t cmd)
+{
+	struct sched_entry *e;
+
+	e = calloc(1, sizeof(*e));
+	if (!e)
+		return NULL;
+
+	e->gatemask = gatemask;
+	e->interval = interval;
+	e->cmd = cmd;
+
+	return e;
+}
+
+static int taprio_parse_opt(struct qdisc_util *qu, int argc,
+			    char **argv, struct nlmsghdr *n, const char *dev)
+{
+	__s32 clockid = CLOCKID_INVALID;
+	struct tc_mqprio_qopt opt = { };
+	struct list_head sched_entries;
+	struct rtattr *tail;
+	__s64 base_time = 0;
+	int err, idx;
+
+	INIT_LIST_HEAD(&sched_entries);
+
+	while (argc > 0) {
+		idx = 0;
+		if (strcmp(*argv, "num_tc") == 0) {
+			NEXT_ARG();
+			if (get_u8(&opt.num_tc, *argv, 10)) {
+				fprintf(stderr, "Illegal \"num_tc\"\n");
+				return -1;
+			}
+		} else if (strcmp(*argv, "map") == 0) {
+			while (idx < TC_QOPT_MAX_QUEUE && NEXT_ARG_OK()) {
+				NEXT_ARG();
+				if (get_u8(&opt.prio_tc_map[idx], *argv, 10)) {
+					PREV_ARG();
+					break;
+				}
+				idx++;
+			}
+			for ( ; idx < TC_QOPT_MAX_QUEUE; idx++)
+				opt.prio_tc_map[idx] = 0;
+		} else if (strcmp(*argv, "queues") == 0) {
+			char *tmp, *tok;
+
+			while (idx < TC_QOPT_MAX_QUEUE && NEXT_ARG_OK()) {
+				NEXT_ARG();
+
+				tmp = strdup(*argv);
+				if (!tmp)
+					break;
+
+				tok = strtok(tmp, "@");
+				if (get_u16(&opt.count[idx], tok, 10)) {
+					free(tmp);
+					PREV_ARG();
+					break;
+				}
+				tok = strtok(NULL, "@");
+				if (get_u16(&opt.offset[idx], tok, 10)) {
+					free(tmp);
+					PREV_ARG();
+					break;
+				}
+				free(tmp);
+				idx++;
+			}
+		} else if (strcmp(*argv, "sched-entry") == 0) {
+			uint32_t mask, interval;
+			struct sched_entry *e;
+			uint8_t cmd;
+
+			NEXT_ARG();
+			err = str_to_entry_cmd(*argv);
+			if (err < 0) {
+				explain_sched_entry();
+				return  -1;
+			}
+			cmd = err;
+
+			NEXT_ARG();
+			if (get_u32(&mask, *argv, 16)) {
+				explain_sched_entry();
+				return -1;
+			}
+
+			NEXT_ARG();
+			if (get_u32(&interval, *argv, 0)) {
+				explain_sched_entry();
+				return -1;
+			}
+
+			e = create_entry(mask, interval, cmd);
+			if (!e) {
+				fprintf(stderr, "taprio: not enough memory for new schedule entry\n");
+				return -1;
+			}
+
+			list_add_tail(&e->list, &sched_entries);
+
+		} else if (strcmp(*argv, "base-time") == 0) {
+			NEXT_ARG();
+			if (get_s64(&base_time, *argv, 10)) {
+				PREV_ARG();
+				break;
+			}
+		} else if (strcmp(*argv, "clockid") == 0) {
+			NEXT_ARG();
+			if (clockid != CLOCKID_INVALID) {
+				fprintf(stderr, "taprio: duplicate \"clockid\" specification\n");
+				return -1;
+			}
+			if (get_clockid(&clockid, *argv)) {
+				explain_clockid(*argv);
+				return -1;
+			}
+		} else if (strcmp(*argv, "help") == 0) {
+			explain();
+			return -1;
+		} else {
+			fprintf(stderr, "Unknown argument\n");
+			return -1;
+		}
+		argc--; argv++;
+	}
+
+	tail = NLMSG_TAIL(n);
+	addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+
+	if (opt.num_tc > 0)
+		addattr_l(n, 1024, TCA_TAPRIO_ATTR_PRIOMAP, &opt, sizeof(opt));
+
+	if (base_time)
+		addattr_l(n, 1024, TCA_TAPRIO_ATTR_SCHED_BASE_TIME, &base_time, sizeof(base_time));
+
+	addattr_l(n, 1024, TCA_TAPRIO_ATTR_SCHED_CLOCKID, &clockid, sizeof(clockid));
+
+	if (!list_empty(&sched_entries)) {
+		struct rtattr *entry_list;
+		entry_list = addattr_nest(n, 1024, TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST | NLA_F_NESTED);
+
+		err = add_sched_list(&sched_entries, n);
+		if (err < 0) {
+			fprintf(stderr, "Could not add schedule to netlink message\n");
+			return -1;
+		}
+
+		addattr_nest_end(n, entry_list);
+	}
+
+	tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+
+	return 0;
+}
+
+static int print_sched_list(FILE *f, struct rtattr *list)
+{
+	struct rtattr *item;
+	int rem;
+
+	if (list == NULL)
+		return 0;
+
+	rem = RTA_PAYLOAD(list);
+
+	open_json_array(PRINT_JSON, "schedule");
+
+	for (item = RTA_DATA(list); RTA_OK(item, rem); item = RTA_NEXT(item, rem)) {
+		struct rtattr *tb[TCA_TAPRIO_SCHED_ENTRY_MAX + 1];
+		__u32 index = 0, gatemask = 0, interval = 0;
+		__u8 command = 0;
+
+		parse_rtattr_nested(tb, TCA_TAPRIO_SCHED_ENTRY_MAX, item);
+
+		if (tb[TCA_TAPRIO_SCHED_ENTRY_INDEX])
+			index = rta_getattr_u32(tb[TCA_TAPRIO_SCHED_ENTRY_INDEX]);
+
+		if (tb[TCA_TAPRIO_SCHED_ENTRY_CMD])
+			command = rta_getattr_u8(tb[TCA_TAPRIO_SCHED_ENTRY_CMD]);
+
+		if (tb[TCA_TAPRIO_SCHED_ENTRY_GATE_MASK])
+			gatemask = rta_getattr_u32(tb[TCA_TAPRIO_SCHED_ENTRY_GATE_MASK]);
+
+		if (tb[TCA_TAPRIO_SCHED_ENTRY_INTERVAL])
+			interval = rta_getattr_u32(tb[TCA_TAPRIO_SCHED_ENTRY_INTERVAL]);
+
+		open_json_object(NULL);
+		print_uint(PRINT_ANY, "index", "\tindex %u", index);
+		print_string(PRINT_ANY, "cmd", " cmd %s", entry_cmd_to_str(command));
+		print_0xhex(PRINT_ANY, "gatemask", " gatemask %#x", gatemask);
+		print_uint(PRINT_ANY, "interval", " interval %u", interval);
+		close_json_object();
+
+		print_string(PRINT_FP, NULL, "%s", _SL_);
+	}
+
+	close_json_array(PRINT_ANY, "");
+
+	return 0;
+}
+
+static int taprio_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
+{
+	struct rtattr *tb[TCA_TAPRIO_ATTR_MAX + 1];
+	struct tc_mqprio_qopt *qopt = 0;
+	__s32 clockid = CLOCKID_INVALID;
+	__s64 base_time = 0;
+	int i;
+
+	if (opt == NULL)
+		return 0;
+
+	parse_rtattr_nested(tb, TCA_TAPRIO_ATTR_MAX, opt);
+
+	if (tb[TCA_TAPRIO_ATTR_PRIOMAP] == NULL)
+		return -1;
+
+	qopt = RTA_DATA(tb[TCA_TAPRIO_ATTR_PRIOMAP]);
+
+	print_uint(PRINT_ANY, "tc", "tc %u ", qopt->num_tc);
+
+	open_json_array(PRINT_ANY, "map");
+	for (i = 0; i <= TC_PRIO_MAX; i++)
+		print_uint(PRINT_ANY, NULL, " %u", qopt->prio_tc_map[i]);
+	close_json_array(PRINT_ANY, "");
+
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+
+	open_json_array(PRINT_ANY, "queues");
+	for (i = 0; i < qopt->num_tc; i++) {
+		open_json_object(NULL);
+		print_uint(PRINT_ANY, "offset", " offset %u", qopt->offset[i]);
+		print_uint(PRINT_ANY, "count", " count %u", qopt->count[i]);
+		close_json_object();
+	}
+	close_json_array(PRINT_ANY, "");
+
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+
+	if (tb[TCA_TAPRIO_ATTR_SCHED_BASE_TIME])
+		base_time = rta_getattr_s64(tb[TCA_TAPRIO_ATTR_SCHED_BASE_TIME]);
+
+	if (tb[TCA_TAPRIO_ATTR_SCHED_CLOCKID])
+		clockid = rta_getattr_s32(tb[TCA_TAPRIO_ATTR_SCHED_CLOCKID]);
+
+	print_string(PRINT_ANY, "clockid", "clockid %s", get_clock_name(clockid));
+
+	print_lluint(PRINT_ANY, "base_time", " base-time %lld", base_time);
+
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+
+	return print_sched_list(f, tb[TCA_TAPRIO_ATTR_SCHED_ENTRY_LIST]);
+}
+
+struct qdisc_util taprio_qdisc_util = {
+	.id		= "taprio",
+	.parse_qopt	= taprio_parse_opt,
+	.print_qopt	= taprio_print_opt,
+};
-- 
2.19.0

^ permalink raw reply related

* Re: [bpf-next PATCH 1/3] net: fix generic XDP to handle if eth header was mangled
From: Song Liu @ 2018-10-04 23:20 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Networking, Daniel Borkmann, yoel, Alexei Starovoitov
In-Reply-To: <20181003160413.755f9cd5@redhat.com>

Hi Jesper,

On Wed, Oct 3, 2018 at 7:04 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
> On Tue, 25 Sep 2018 22:36:39 -0700
> Song Liu <liu.song.a23@gmail.com> wrote:
>
> > On Tue, Sep 25, 2018 at 7:26 AM Jesper Dangaard Brouer
> > <brouer@redhat.com> wrote:
> > >
> > > XDP can modify (and resize) the Ethernet header in the packet.
> > >
> > > There is a bug in generic-XDP, because skb->protocol and skb->pkt_type
> > > are setup before reaching (netif_receive_)generic_xdp.
> > >
> > > This bug was hit when XDP were popping VLAN headers (changing
> > > eth->h_proto), as skb->protocol still contains VLAN-indication
> > > (ETH_P_8021Q) causing invocation of skb_vlan_untag(skb), which corrupt
> > > the packet (basically popping the VLAN again).
> > >
> > > This patch catch if XDP changed eth header in such a way, that SKB
> > > fields needs to be updated.
> > >
> > > Fixes: d445516966dc ("net: xdp: support xdp generic on virtual devices")
> > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > > ---
> > >  net/core/dev.c |   14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index ca78dc5a79a3..db6d89f536cb 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -4258,6 +4258,9 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> > >         struct netdev_rx_queue *rxqueue;
> > >         void *orig_data, *orig_data_end;
> > >         u32 metalen, act = XDP_DROP;
> > > +       __be16 orig_eth_type;
> > > +       struct ethhdr *eth;
> > > +       bool orig_bcast;
> > >         int hlen, off;
> > >         u32 mac_len;
> > >
> > > @@ -4298,6 +4301,9 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> > >         xdp->data_hard_start = skb->data - skb_headroom(skb);
> > >         orig_data_end = xdp->data_end;
> > >         orig_data = xdp->data;
> > > +       eth = (struct ethhdr *)xdp->data;
> > > +       orig_bcast = is_multicast_ether_addr_64bits(eth->h_dest);
> > > +       orig_eth_type = eth->h_proto;
> > >
> > >         rxqueue = netif_get_rxqueue(skb);
> > >         xdp->rxq = &rxqueue->xdp_rxq;
> > > @@ -4321,6 +4327,14 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> > >
> > >         }
> > >
> > > +       /* check if XDP changed eth hdr such SKB needs update */
> > > +       eth = (struct ethhdr *)xdp->data;
> > > +       if ((orig_eth_type != eth->h_proto) ||
> > > +           (orig_bcast != is_multicast_ether_addr_64bits(eth->h_dest))) {
> >
> > Is the actions below always correct for the condition above? Do we need
> > to confirm the SKB is updated properly?
>
> I cannot find the issue that you are hinting to?
>
> If the BPF prog used bpf_xdp_adjust_head(), which the included selftest
> program does, then skb->data have been appropriately adjusted just
> above (with __skb_pull(skb, off) or __skb_push(skb, -off)), which makes
> the call to skb_reset_mac_header(skb) inside eth_type_trans() correct.
>
> I've double checked the code, and I cannot find anything wrong...
> please let me know if I missed something!?
>
>
> > > +               __skb_push(skb, mac_len);
> > > +               skb->protocol = eth_type_trans(skb, skb->dev);
>
> We could change mac_len to be ETH_HLEN, because inside eth_type_trans()
> the constant ETH_HLEN is used, that way we are 100% sure the
> skb_push/skb_pull are "paired".  Will that be better for you?

Thanks for the explanation. Now I understand it better.
I think using ETH_HLEN is better. Both napi_frags_finish() and
__dev_forward_skb()
use similar pattern.

Song

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox