netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback
  2018-05-28 14:27 [PATCH net-next v16 0/8] sched: Add Common Applications Kept Enhanced (cake) qdisc Toke Høiland-Jørgensen
  2018-05-28 14:27 ` [PATCH net-next v16 5/8] sch_cake: Add NAT awareness to packet classifier Toke Høiland-Jørgensen
@ 2018-05-28 14:27 ` Toke Høiland-Jørgensen
  2018-05-28 19:49   ` Pablo Neira Ayuso
                     ` (2 more replies)
  1 sibling, 3 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-05-28 14:27 UTC (permalink / raw)
  To: netdev, cake; +Cc: netfilter-devel

This adds a callback to netfilter to extract a conntrack tuple from an skb
that works before the _nfct skb field has been initialised (e.g., in an
ingress qdisc). The tuple is copied to the caller to avoid issues with
reference counting.

The callback will return false when conntrack is not loaded, allowing it to
be accessed without incurring a module dependency on conntrack. This is
used by the NAT mode in sch_cake.

Cc: netfilter-devel@vger.kernel.org
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
 include/linux/netfilter.h         |    6 ++++++
 net/netfilter/core.c              |   21 +++++++++++++++++++++
 net/netfilter/nf_conntrack_core.c |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 85a1a0b32c66..7cbe7e9ce527 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -375,6 +375,12 @@ nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl, u_int8_t family)
 extern void (*ip_ct_attach)(struct sk_buff *, const struct sk_buff *) __rcu;
 void nf_ct_attach(struct sk_buff *, const struct sk_buff *);
 extern void (*nf_ct_destroy)(struct nf_conntrack *) __rcu;
+
+struct nf_conntrack_tuple;
+extern bool (*skb_ct_get_tuple)(struct nf_conntrack_tuple *,
+				const struct sk_buff *) __rcu;
+bool nf_ct_get_tuple_skb(struct nf_conntrack_tuple *dst_tuple,
+			 const struct sk_buff *skb);
 #else
 static inline void nf_ct_attach(struct sk_buff *new, struct sk_buff *skb) {}
 #endif
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 0f6b8172fb9a..520565198f0e 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -572,6 +572,27 @@ void nf_conntrack_destroy(struct nf_conntrack *nfct)
 }
 EXPORT_SYMBOL(nf_conntrack_destroy);
 
+bool (*skb_ct_get_tuple)(struct nf_conntrack_tuple *,
+			 const struct sk_buff *) __rcu __read_mostly;
+EXPORT_SYMBOL(skb_ct_get_tuple);
+
+bool nf_ct_get_tuple_skb(struct nf_conntrack_tuple *dst_tuple,
+			 const struct sk_buff *skb)
+{
+	bool (*get_tuple)(const struct sk_buff *, struct nf_conntrack_tuple *);
+	bool ret = false;
+
+	rcu_read_lock();
+	get_tuple = rcu_dereference(skb_ct_get_tuple);
+	if (!get_tuple)
+		goto out;
+	ret = get_tuple(dst_tuple, skb);
+out:
+	rcu_read_unlock();
+	return ret;
+}
+EXPORT_SYMBOL(nf_ct_get_tuple_skb);
+
 /* Built-in default zone used e.g. by modules. */
 const struct nf_conntrack_zone nf_ct_zone_dflt = {
 	.id	= NF_CT_DEFAULT_ZONE_ID,
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 41ff04ee2554..eee5c76f638c 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1611,6 +1611,41 @@ static void nf_conntrack_attach(struct sk_buff *nskb, const struct sk_buff *skb)
 	nf_conntrack_get(skb_nfct(nskb));
 }
 
+static bool nf_conntrack_get_tuple_skb(struct nf_conntrack_tuple *dst_tuple,
+				       const struct sk_buff *skb)
+{
+	const struct nf_conntrack_tuple *src_tuple;
+	const struct nf_conntrack_tuple_hash *hash;
+	struct nf_conntrack_tuple srctuple;
+	enum ip_conntrack_info ctinfo;
+	struct nf_conn *ct;
+
+	ct = nf_ct_get(skb, &ctinfo);
+	if (ct) {
+		src_tuple = nf_ct_tuple(ct, CTINFO2DIR(ctinfo));
+		memcpy(dst_tuple, src_tuple, sizeof(*dst_tuple));
+		return true;
+	}
+
+	if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
+			       NFPROTO_IPV4, dev_net(skb->dev),
+			       &srctuple))
+		return false;
+
+	hash = nf_conntrack_find_get(dev_net(skb->dev),
+				     &nf_ct_zone_dflt,
+				     &srctuple);
+	if (!hash)
+		return false;
+
+	ct = nf_ct_tuplehash_to_ctrack(hash);
+	src_tuple = nf_ct_tuple(ct, !hash->tuple.dst.dir);
+	memcpy(dst_tuple, src_tuple, sizeof(*dst_tuple));
+	nf_ct_put(ct);
+
+	return true;
+}
+
 /* Bring out ya dead! */
 static struct nf_conn *
 get_next_corpse(int (*iter)(struct nf_conn *i, void *data),
@@ -1808,6 +1843,7 @@ void nf_conntrack_cleanup_start(void)
 {
 	conntrack_gc_work.exiting = true;
 	RCU_INIT_POINTER(ip_ct_attach, NULL);
+	RCU_INIT_POINTER(skb_ct_get_tuple, NULL);
 }
 
 void nf_conntrack_cleanup_end(void)
@@ -2135,6 +2171,7 @@ void nf_conntrack_init_end(void)
 	/* For use by REJECT target */
 	RCU_INIT_POINTER(ip_ct_attach, nf_conntrack_attach);
 	RCU_INIT_POINTER(nf_ct_destroy, destroy_conntrack);
+	RCU_INIT_POINTER(skb_ct_get_tuple, nf_conntrack_get_tuple_skb);
 }
 
 /*

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next v16 5/8] sch_cake: Add NAT awareness to packet classifier
  2018-05-28 14:27 [PATCH net-next v16 0/8] sched: Add Common Applications Kept Enhanced (cake) qdisc Toke Høiland-Jørgensen
@ 2018-05-28 14:27 ` Toke Høiland-Jørgensen
  2018-05-28 19:51   ` Pablo Neira Ayuso
  2018-05-28 14:27 ` [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback Toke Høiland-Jørgensen
  1 sibling, 1 reply; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-05-28 14:27 UTC (permalink / raw)
  To: netdev, cake; +Cc: netfilter-devel

When CAKE is deployed on a gateway that also performs NAT (which is a
common deployment mode), the host fairness mechanism cannot distinguish
internal hosts from each other, and so fails to work correctly.

To fix this, we add an optional NAT awareness mode, which will query the
kernel conntrack mechanism to obtain the pre-NAT addresses for each packet
and use that in the flow and host hashing.

When the shaper is enabled and the host is already performing NAT, the cost
of this lookup is negligible. However, in unlimited mode with no NAT being
performed, there is a significant CPU cost at higher bandwidths. For this
reason, the feature is turned off by default.

Cc: netfilter-devel@vger.kernel.org
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
 net/sched/sch_cake.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
index 68ac908470f1..fecd9caac0cc 100644
--- a/net/sched/sch_cake.c
+++ b/net/sched/sch_cake.c
@@ -71,6 +71,10 @@
 #include <net/tcp.h>
 #include <net/flow_dissector.h>
 
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+#include <net/netfilter/nf_conntrack_core.h>
+#endif
+
 #define CAKE_SET_WAYS (8)
 #define CAKE_MAX_TINS (8)
 #define CAKE_QUEUES (1024)
@@ -516,6 +520,29 @@ static bool cobalt_should_drop(struct cobalt_vars *vars,
 	return drop;
 }
 
+static void cake_update_flowkeys(struct flow_keys *keys,
+				 const struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+	struct nf_conntrack_tuple tuple = {};
+	bool rev = !skb->_nfct;
+
+	if (tc_skb_protocol(skb) != htons(ETH_P_IP))
+		return;
+
+	if (!nf_ct_get_tuple_skb(&tuple, skb))
+		return;
+
+	keys->addrs.v4addrs.src = rev ? tuple.dst.u3.ip : tuple.src.u3.ip;
+	keys->addrs.v4addrs.dst = rev ? tuple.src.u3.ip : tuple.dst.u3.ip;
+
+	if (keys->ports.ports) {
+		keys->ports.src = rev ? tuple.dst.u.all : tuple.src.u.all;
+		keys->ports.dst = rev ? tuple.src.u.all : tuple.dst.u.all;
+	}
+#endif
+}
+
 /* Cake has several subtle multiple bit settings. In these cases you
  *  would be matching triple isolate mode as well.
  */
@@ -543,6 +570,9 @@ static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb,
 	skb_flow_dissect_flow_keys(skb, &keys,
 				   FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
 
+	if (flow_mode & CAKE_FLOW_NAT_FLAG)
+		cake_update_flowkeys(&keys, skb);
+
 	/* flow_hash_from_keys() sorts the addresses by value, so we have
 	 * to preserve their order in a separate data structure to treat
 	 * src and dst host addresses as independently selectable.
@@ -1919,6 +1949,18 @@ static int cake_change(struct Qdisc *sch, struct nlattr *opt,
 	if (err < 0)
 		return err;
 
+	if (tb[TCA_CAKE_NAT]) {
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+		q->flow_mode &= ~CAKE_FLOW_NAT_FLAG;
+		q->flow_mode |= CAKE_FLOW_NAT_FLAG *
+			!!nla_get_u32(tb[TCA_CAKE_NAT]);
+#else
+		NL_SET_ERR_MSG_ATTR(extack, "No conntrack support in kernel",
+				    tb[TCA_CAKE_NAT]);
+		return -EOPNOTSUPP;
+#endif
+	}
+
 	if (tb[TCA_CAKE_BASE_RATE64])
 		q->rate_bps = nla_get_u64(tb[TCA_CAKE_BASE_RATE64]);
 
@@ -2091,6 +2133,10 @@ static int cake_dump(struct Qdisc *sch, struct sk_buff *skb)
 	if (nla_put_u32(skb, TCA_CAKE_ACK_FILTER, q->ack_filter))
 		goto nla_put_failure;
 
+	if (nla_put_u32(skb, TCA_CAKE_NAT,
+			!!(q->flow_mode & CAKE_FLOW_NAT_FLAG)))
+		goto nla_put_failure;
+
 	return nla_nest_end(skb, opts);
 
 nla_put_failure:

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next v16 0/8] sched: Add Common Applications Kept Enhanced (cake) qdisc
@ 2018-05-28 14:27 Toke Høiland-Jørgensen
  2018-05-28 14:27 ` [PATCH net-next v16 5/8] sch_cake: Add NAT awareness to packet classifier Toke Høiland-Jørgensen
  2018-05-28 14:27 ` [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback Toke Høiland-Jørgensen
  0 siblings, 2 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-05-28 14:27 UTC (permalink / raw)
  To: netdev, cake
  Cc: Georgios Amanakis, Pete Heist, Yuchung Cheng, Neal Cardwell,
	Dave Taht, netfilter-devel

This patch series adds the CAKE qdisc, and has been split up to ease
review.

I have attempted to split out each configurable feature into its own patch.
The first commit adds the base shaper and packet scheduler, while
subsequent commits add the optional features. The full userspace API and
most data structures are included in this commit, but options not
understood in the base version will be ignored.

The result of applying the entire series is identical to the out of tree
version that have seen extensive testing in previous deployments, most
notably as an out of tree patch to OpenWrt. However, note that I have only
compile tested the individual patches; so the whole series should be
considered as a unit.

---
Changelog

v16:
  - Move conntrack lookup function into conntrack core and read it via
    RCU so it is only active when the nf_conntrack module is loaded.
    This avoids the module dependency on conntrack for NAT mode. Thanks
    to Pablo for the idea.

v15:
  - Handle ECN flags in ACK filter

v14:
  - Handle seqno wraps and DSACKs in ACK filter

v13:
  - Avoid ktime_t to scalar compares
  - Add class dumping and basic stats
  - Fail with ENOTSUPP when requesting NAT mode and conntrack is not
    available.
  - Parse all TCP options in ACK filter and make sure to only drop safe
    ones. Also handle SACK ranges properly.

v12:
  - Get rid of custom time typedefs. Use ktime_t for time and u64 for
    duration instead.

v11:
  - Fix overhead compensation calculation for GSO packets
  - Change configured rate to be u64 (I ran out of bits before I ran out
    of CPU when testing the effects of the above)

v10:
  - Christmas tree gardening (fix variable declarations to be in reverse
    line length order)

v9:
  - Remove duplicated checks around kvfree() and just call it
    unconditionally.
  - Don't pass __GFP_NOWARN when allocating memory
  - Move options in cake_dump() that are related to optional features to
    later patches implementing the features.
  - Support attaching filters to the qdisc and use the classification
    result to select flow queue.
  - Support overriding diffserv priority tin from skb->priority

v8:
  - Remove inline keyword from function definitions
  - Simplify ACK filter; remove the complex state handling to make the
    logic easier to follow. This will potentially be a bit less efficient,
    but I have not been able to measure a difference.

v7:
  - Split up patch into a series to ease review.
  - Constify the ACK filter.

v6:
  - Fix 6in4 encapsulation checks in ACK filter code
  - Checkpatch fixes

v5:
  - Refactor ACK filter code and hopefully fix the safety issues
    properly this time.

v4:
  - Only split GSO packets if shaping at speeds <= 1Gbps
  - Fix overhead calculation code to also work for GSO packets
  - Don't re-implement kvzalloc()
  - Remove local header include from out-of-tree build (fixes kbuild-bot
    complaint).
  - Several fixes to the ACK filter:
    - Check pskb_may_pull() before deref of transport headers.
    - Don't run ACK filter logic on split GSO packets
    - Fix TCP sequence number compare to deal with wraparounds

v3:
  - Use IS_REACHABLE() macro to fix compilation when sch_cake is
    built-in and conntrack is a module.
  - Switch the stats output to use nested netlink attributes instead
    of a versioned struct.
  - Remove GPL boilerplate.
  - Fix array initialisation style.

v2:
  - Fix kbuild test bot complaint
  - Clean up the netlink ABI
  - Fix checkpatch complaints
  - A few tweaks to the behaviour of cake based on testing carried out
    while writing the paper.


---

Toke Høiland-Jørgensen (8):
      sched: Add Common Applications Kept Enhanced (cake) qdisc
      sch_cake: Add ingress mode
      sch_cake: Add optional ACK filter
      netfilter: Add nf_ct_get_tuple_skb callback
      sch_cake: Add NAT awareness to packet classifier
      sch_cake: Add DiffServ handling
      sch_cake: Add overhead compensation support to the rate shaper
      sch_cake: Conditionally split GSO segments


 include/linux/netfilter.h         |    6 
 include/uapi/linux/pkt_sched.h    |  113 +
 net/netfilter/core.c              |   21 
 net/netfilter/nf_conntrack_core.c |   37 
 net/sched/Kconfig                 |   11 
 net/sched/Makefile                |    1 
 net/sched/sch_cake.c              | 2987 +++++++++++++++++++++++++++++++++++++
 7 files changed, 3176 insertions(+)
 create mode 100644 net/sched/sch_cake.c

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback
  2018-05-28 14:27 ` [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback Toke Høiland-Jørgensen
@ 2018-05-28 19:49   ` Pablo Neira Ayuso
  2018-05-28 21:28     ` Toke Høiland-Jørgensen
  2018-05-30  6:11   ` kbuild test robot
  2018-05-30  8:33   ` kbuild test robot
  2 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 19:49 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: netdev, cake, netfilter-devel

On Mon, May 28, 2018 at 04:27:46PM +0200, Toke Høiland-Jørgensen wrote:
[...]
> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> index 0f6b8172fb9a..520565198f0e 100644
> --- a/net/netfilter/core.c
> +++ b/net/netfilter/core.c
> @@ -572,6 +572,27 @@ void nf_conntrack_destroy(struct nf_conntrack *nfct)
>  }
>  EXPORT_SYMBOL(nf_conntrack_destroy);
>  
> +bool (*skb_ct_get_tuple)(struct nf_conntrack_tuple *,
> +			 const struct sk_buff *) __rcu __read_mostly;
> +EXPORT_SYMBOL(skb_ct_get_tuple);

Now we have struct nf_ct_hook in net-next, please add ->get_tuple to
that new object.

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v16 5/8] sch_cake: Add NAT awareness to packet classifier
  2018-05-28 14:27 ` [PATCH net-next v16 5/8] sch_cake: Add NAT awareness to packet classifier Toke Høiland-Jørgensen
@ 2018-05-28 19:51   ` Pablo Neira Ayuso
  2018-05-28 22:19     ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 9+ messages in thread
From: Pablo Neira Ayuso @ 2018-05-28 19:51 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen; +Cc: netdev, cake, netfilter-devel

On Mon, May 28, 2018 at 04:27:46PM +0200, Toke Høiland-Jørgensen wrote:
> When CAKE is deployed on a gateway that also performs NAT (which is a
> common deployment mode), the host fairness mechanism cannot distinguish
> internal hosts from each other, and so fails to work correctly.
> 
> To fix this, we add an optional NAT awareness mode, which will query the
> kernel conntrack mechanism to obtain the pre-NAT addresses for each packet
> and use that in the flow and host hashing.
> 
> When the shaper is enabled and the host is already performing NAT, the cost
> of this lookup is negligible. However, in unlimited mode with no NAT being
> performed, there is a significant CPU cost at higher bandwidths. For this
> reason, the feature is turned off by default.
> 
> Cc: netfilter-devel@vger.kernel.org
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> ---
>  net/sched/sch_cake.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
> index 68ac908470f1..fecd9caac0cc 100644
> --- a/net/sched/sch_cake.c
> +++ b/net/sched/sch_cake.c
> @@ -71,6 +71,10 @@
>  #include <net/tcp.h>
>  #include <net/flow_dissector.h>
>  
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
> +#include <net/netfilter/nf_conntrack_core.h>
> +#endif
> +
>  #define CAKE_SET_WAYS (8)
>  #define CAKE_MAX_TINS (8)
>  #define CAKE_QUEUES (1024)
> @@ -516,6 +520,29 @@ static bool cobalt_should_drop(struct cobalt_vars *vars,
>  	return drop;
>  }
>  
> +static void cake_update_flowkeys(struct flow_keys *keys,
> +				 const struct sk_buff *skb)
> +{
> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)

I would remove the ifdef, not really needed, it will simplify things.

But I leave it to you to decide, this is not I deal breaker.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback
  2018-05-28 19:49   ` Pablo Neira Ayuso
@ 2018-05-28 21:28     ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-05-28 21:28 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netdev, cake, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> writes:

> On Mon, May 28, 2018 at 04:27:46PM +0200, Toke Høiland-Jørgensen wrote:
> [...]
>> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
>> index 0f6b8172fb9a..520565198f0e 100644
>> --- a/net/netfilter/core.c
>> +++ b/net/netfilter/core.c
>> @@ -572,6 +572,27 @@ void nf_conntrack_destroy(struct nf_conntrack *nfct)
>>  }
>>  EXPORT_SYMBOL(nf_conntrack_destroy);
>>  
>> +bool (*skb_ct_get_tuple)(struct nf_conntrack_tuple *,
>> +			 const struct sk_buff *) __rcu __read_mostly;
>> +EXPORT_SYMBOL(skb_ct_get_tuple);
>
> Now we have struct nf_ct_hook in net-next, please add ->get_tuple to
> that new object.

Ah, right, will do :)

-Toke

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v16 5/8] sch_cake: Add NAT awareness to packet classifier
  2018-05-28 19:51   ` Pablo Neira Ayuso
@ 2018-05-28 22:19     ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 9+ messages in thread
From: Toke Høiland-Jørgensen @ 2018-05-28 22:19 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netdev, cake, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> writes:

> On Mon, May 28, 2018 at 04:27:46PM +0200, Toke Høiland-Jørgensen wrote:
>> When CAKE is deployed on a gateway that also performs NAT (which is a
>> common deployment mode), the host fairness mechanism cannot distinguish
>> internal hosts from each other, and so fails to work correctly.
>> 
>> To fix this, we add an optional NAT awareness mode, which will query the
>> kernel conntrack mechanism to obtain the pre-NAT addresses for each packet
>> and use that in the flow and host hashing.
>> 
>> When the shaper is enabled and the host is already performing NAT, the cost
>> of this lookup is negligible. However, in unlimited mode with no NAT being
>> performed, there is a significant CPU cost at higher bandwidths. For this
>> reason, the feature is turned off by default.
>> 
>> Cc: netfilter-devel@vger.kernel.org
>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> ---
>>  net/sched/sch_cake.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>> 
>> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c
>> index 68ac908470f1..fecd9caac0cc 100644
>> --- a/net/sched/sch_cake.c
>> +++ b/net/sched/sch_cake.c
>> @@ -71,6 +71,10 @@
>>  #include <net/tcp.h>
>>  #include <net/flow_dissector.h>
>>  
>> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
>> +#include <net/netfilter/nf_conntrack_core.h>
>> +#endif
>> +
>>  #define CAKE_SET_WAYS (8)
>>  #define CAKE_MAX_TINS (8)
>>  #define CAKE_QUEUES (1024)
>> @@ -516,6 +520,29 @@ static bool cobalt_should_drop(struct cobalt_vars *vars,
>>  	return drop;
>>  }
>>  
>> +static void cake_update_flowkeys(struct flow_keys *keys,
>> +				 const struct sk_buff *skb)
>> +{
>> +#if IS_ENABLED(CONFIG_NF_CONNTRACK)
>
> I would remove the ifdef, not really needed, it will simplify things.
>
> But I leave it to you to decide, this is not I deal breaker.

If I remove it I get a bunch of 'incomplete type' errors when compiling.
Besides, we use it to report an error to userspace when conntrack is
disabled anyway, so might as well keep the whole thing ifdef'ed.

-Toke

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback
  2018-05-28 14:27 ` [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback Toke Høiland-Jørgensen
  2018-05-28 19:49   ` Pablo Neira Ayuso
@ 2018-05-30  6:11   ` kbuild test robot
  2018-05-30  8:33   ` kbuild test robot
  2 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-05-30  6:11 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: kbuild-all, netdev, cake, netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]

Hi Toke,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]
[also build test WARNING on v4.17-rc7]
[cannot apply to net-next/master next-20180529]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/sched-Add-Common-Applications-Kept-Enhanced-cake-qdisc/20180530-125240
config: i386-randconfig-a0-05291352 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   net/netfilter/core.c: In function 'nf_ct_get_tuple_skb':
>> net/netfilter/core.c:586:12: warning: assignment from incompatible pointer type
     get_tuple = rcu_dereference(skb_ct_get_tuple);
               ^
>> net/netfilter/core.c:589:18: warning: passing argument 1 of 'get_tuple' from incompatible pointer type
     ret = get_tuple(dst_tuple, skb);
                     ^
   net/netfilter/core.c:589:18: note: expected 'const struct sk_buff *' but argument is of type 'struct nf_conntrack_tuple *'
   net/netfilter/core.c:589:29: warning: passing argument 2 of 'get_tuple' from incompatible pointer type
     ret = get_tuple(dst_tuple, skb);
                                ^
   net/netfilter/core.c:589:29: note: expected 'struct nf_conntrack_tuple *' but argument is of type 'const struct sk_buff *'

vim +586 net/netfilter/core.c

   578	
   579	bool nf_ct_get_tuple_skb(struct nf_conntrack_tuple *dst_tuple,
   580				 const struct sk_buff *skb)
   581	{
   582		bool (*get_tuple)(const struct sk_buff *, struct nf_conntrack_tuple *);
   583		bool ret = false;
   584	
   585		rcu_read_lock();
 > 586		get_tuple = rcu_dereference(skb_ct_get_tuple);
   587		if (!get_tuple)
   588			goto out;
 > 589		ret = get_tuple(dst_tuple, skb);
   590	out:
   591		rcu_read_unlock();
   592		return ret;
   593	}
   594	EXPORT_SYMBOL(nf_ct_get_tuple_skb);
   595	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28923 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback
  2018-05-28 14:27 ` [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback Toke Høiland-Jørgensen
  2018-05-28 19:49   ` Pablo Neira Ayuso
  2018-05-30  6:11   ` kbuild test robot
@ 2018-05-30  8:33   ` kbuild test robot
  2 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2018-05-30  8:33 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: kbuild-all, netdev, cake, netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 2346 bytes --]

Hi Toke,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on v4.17-rc7]
[cannot apply to net-next/master next-20180529]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Toke-H-iland-J-rgensen/sched-Add-Common-Applications-Kept-Enhanced-cake-qdisc/20180530-125240
config: x86_64-randconfig-x015-201821 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net/netfilter/core.c: In function 'nf_ct_get_tuple_skb':
>> net/netfilter/core.c:586:12: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
     get_tuple = rcu_dereference(skb_ct_get_tuple);
               ^
>> net/netfilter/core.c:589:18: error: passing argument 1 of 'get_tuple' from incompatible pointer type [-Werror=incompatible-pointer-types]
     ret = get_tuple(dst_tuple, skb);
                     ^~~~~~~~~
   net/netfilter/core.c:589:18: note: expected 'const struct sk_buff *' but argument is of type 'struct nf_conntrack_tuple *'
   net/netfilter/core.c:589:29: error: passing argument 2 of 'get_tuple' from incompatible pointer type [-Werror=incompatible-pointer-types]
     ret = get_tuple(dst_tuple, skb);
                                ^~~
   net/netfilter/core.c:589:29: note: expected 'struct nf_conntrack_tuple *' but argument is of type 'const struct sk_buff *'
   cc1: some warnings being treated as errors

vim +586 net/netfilter/core.c

   578	
   579	bool nf_ct_get_tuple_skb(struct nf_conntrack_tuple *dst_tuple,
   580				 const struct sk_buff *skb)
   581	{
   582		bool (*get_tuple)(const struct sk_buff *, struct nf_conntrack_tuple *);
   583		bool ret = false;
   584	
   585		rcu_read_lock();
 > 586		get_tuple = rcu_dereference(skb_ct_get_tuple);
   587		if (!get_tuple)
   588			goto out;
 > 589		ret = get_tuple(dst_tuple, skb);
   590	out:
   591		rcu_read_unlock();
   592		return ret;
   593	}
   594	EXPORT_SYMBOL(nf_ct_get_tuple_skb);
   595	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28422 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-05-30  8:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-28 14:27 [PATCH net-next v16 0/8] sched: Add Common Applications Kept Enhanced (cake) qdisc Toke Høiland-Jørgensen
2018-05-28 14:27 ` [PATCH net-next v16 5/8] sch_cake: Add NAT awareness to packet classifier Toke Høiland-Jørgensen
2018-05-28 19:51   ` Pablo Neira Ayuso
2018-05-28 22:19     ` Toke Høiland-Jørgensen
2018-05-28 14:27 ` [PATCH net-next v16 4/8] netfilter: Add nf_ct_get_tuple_skb callback Toke Høiland-Jørgensen
2018-05-28 19:49   ` Pablo Neira Ayuso
2018-05-28 21:28     ` Toke Høiland-Jørgensen
2018-05-30  6:11   ` kbuild test robot
2018-05-30  8:33   ` kbuild test robot

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).