* Re: [rdma 1/1] RDMA/irdma: Add Kconfig and Makefile
From: Jason Gunthorpe @ 2019-07-04 12:18 UTC (permalink / raw)
To: Jeff Kirsher
Cc: dledford@redhat.com, davem@davemloft.net, Shiraz Saleem,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
david.m.ertman@intel.com, mustafa.ismail@intel.com
In-Reply-To: <20190704021259.15489-2-jeffrey.t.kirsher@intel.com>
On Wed, Jul 03, 2019 at 07:12:43PM -0700, Jeff Kirsher wrote:
> From: Shiraz Saleem <shiraz.saleem@intel.com>
>
> Add Kconfig and Makefile to build irdma driver and mark i40iw
> deprecated/obsolete, since the irdma driver is replacing it and supports
> x722 devices.
Patch 1/1? Series looks mangled...
> diff --git a/drivers/infiniband/hw/i40iw/Kconfig b/drivers/infiniband/hw/i40iw/Kconfig
> index d867ef1ac72a..7454b84b74be 100644
> +++ b/drivers/infiniband/hw/i40iw/Kconfig
> @@ -1,8 +1,10 @@
> config INFINIBAND_I40IW
> - tristate "Intel(R) Ethernet X722 iWARP Driver"
> + tristate "Intel(R) Ethernet X722 iWARP Driver (DEPRECATED)"
> depends on INET && I40E
> depends on IPV6 || !IPV6
> depends on PCI
> + depends on !(INFINBAND_IRDMA=y || INFINIBAND_IRDMA=m)
No.. all drivers must be able to build at once. At least add some
COMPILE_TEST in here to enable building.
Jason
^ permalink raw reply
* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Michal Kubecek @ 2019-07-04 12:17 UTC (permalink / raw)
To: netdev
Cc: Johannes Berg, Jiri Pirko, David Miller, Jakub Kicinski,
Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger,
linux-kernel
In-Reply-To: <6c070d62ffe342f5bc70556ef0f85740d04ae4a3.camel@sipsolutions.net>
On Thu, Jul 04, 2019 at 02:03:02PM +0200, Johannes Berg wrote:
> On Thu, 2019-07-04 at 13:52 +0200, Michal Kubecek wrote:
> >
> > There is still the question if it it should be implemented as a nested
> > attribute which could look like the current compact form without the
> > "list" flag (if there is no mask, it's a list). Or an unstructured data
> > block consisting of u32 bit length
>
> You wouldn't really need the length, since the attribute has a length
> already :-)
It has byte length, not bit length. The bitmaps we are dealing with
can have any bit length, not necessarily multiples of 8 (or even 32).
Michal
^ permalink raw reply
* Re: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Jason Gunthorpe @ 2019-07-04 12:16 UTC (permalink / raw)
To: Jeff Kirsher, Greg KH
Cc: davem@davemloft.net, dledford@redhat.com, Tony Nguyen,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
mustafa.ismail@intel.com, shiraz.saleem@intel.com, Dave Ertman,
Andrew Bowers
In-Reply-To: <20190704021252.15534-2-jeffrey.t.kirsher@intel.com>
On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> From: Tony Nguyen <anthony.l.nguyen@intel.com>
>
> The RDMA block does not advertise on the PCI bus or any other bus.
> Thus the ice driver needs to provide access to the RDMA hardware block
> via a virtual bus; utilize the platform bus to provide this access.
>
> This patch initializes the driver to support RDMA as well as creates
> and registers a platform device for the RDMA driver to register to. At
> this point the driver is fully initialized to register a platform
> driver, however, can not yet register as the ops have not been
> implemented.
I think you need Greg's ack on all this driver stuff - particularly
that a platform_device is OK.
Jason
^ permalink raw reply
* Re: [net-next 0/3][pull request] Intel Wired LAN ver Updates 2019-07-03
From: Jason Gunthorpe @ 2019-07-04 12:15 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem@davemloft.net, dledford@redhat.com, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, nhorman@redhat.com,
sassmann@redhat.com, mustafa.ismail@intel.com,
shiraz.saleem@intel.com, david.m.ertman@intel.com
In-Reply-To: <20190704021252.15534-1-jeffrey.t.kirsher@intel.com>
On Wed, Jul 03, 2019 at 07:12:49PM -0700, Jeff Kirsher wrote:
> This series contains updates to i40e an ice drivers only and is required
> for a series of changes being submitted to the RDMA maintainer/tree.
> Vice Versa, the Intel RDMA driver patches could not be applied to
> net-next due to dependencies to the changes currently in the for-next
> branch of the rdma git tree.
RDMA driver code is not to be applied to the net-next tree. We've
learned this causes too many work flow problems.
You must co-ordinate your driver with a shared git tree as Mellanox is
doing, or wait for alternating kernel releases.
I'm not sure what to do with this PR, it is far too late in the cycle
to submit a new driver so most likely net should not go ahead with
this prep work until this new driver model scheme is properly
reviewed.
> The following are changes since commit a51df9f8da43e8bf9e508143630849b7d696e053:
> gve: fix -ENOMEM null check on a page allocation
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 100GbE
And if you expect anything to be shared with rdma you need to produce
pull requests that are based on some sensible -rc tag.
Jason
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Arnd Bergmann @ 2019-07-04 12:14 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Jesper Dangaard Brouer, Jose Abreu, Joao Pinto, Alexandre Torgue,
Maxime Ripard, Networking, Linux Kernel Mailing List,
David S . Miller, Chen-Yu Tsai, Maxime Coquelin,
Giuseppe Cavallaro, linux-stm32, Linux ARM
In-Reply-To: <20190704103057.GA29734@apalos>
On Thu, Jul 4, 2019 at 12:31 PM Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
> > On Wed, 3 Jul 2019 12:37:50 +0200
> > Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> 1. page pool allocs packet. The API doesn't sync but i *think* you don't have to
> explicitly since the CPU won't touch that buffer until the NAPI handler kicks
> in. On the napi handler you need to dma_sync_single_for_cpu() and process the
> packet.
> So bvottom line i *think* we can skip the dma_sync_single_for_device() on the
> initial allocation *only*. If am terribly wrong please let me know :)
I think you have to do a sync_single_for_device /somewhere/ before the
buffer is given to the device. On a non-cache-coherent machine with
a write-back cache, there may be dirty cache lines that get written back
after the device DMA's data into it (e.g. from a previous memset
from before the buffer got freed), so you absolutely need to flush any
dirty cache lines on it first.
You may also need to invalidate the cache lines in the following
sync_single_for_cpu() to eliminate clean cache lines with stale data
that got there when speculatively reading between the cache-invalidate
and the DMA.
Arnd
^ permalink raw reply
* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Johannes Berg @ 2019-07-04 12:07 UTC (permalink / raw)
To: Jiri Pirko
Cc: Michal Kubecek, David Miller, netdev, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, linux-kernel
In-Reply-To: <20190703143724.GD2250@nanopsycho>
On Wed, 2019-07-03 at 16:37 +0200, Jiri Pirko wrote:
> Wed, Jul 03, 2019 at 03:44:57PM CEST, johannes@sipsolutions.net wrote:
> > On Wed, 2019-07-03 at 13:49 +0200, Jiri Pirko wrote:
> > >
> > > > +Value and mask must have length at least ETHTOOL_A_BITSET_SIZE bits rounded up
> > > > +to a multiple of 32 bits. They consist of 32-bit words in host byte order,
> > >
> > > Looks like the blocks are similar to NLA_BITFIELD32. Why don't you user
> > > nested array of NLA_BITFIELD32 instead?
> >
> > That would seem kind of awkward to use, IMHO.
> >
> > Perhaps better to make some kind of generic "arbitrary size bitfield"
> > attribute type?
>
> Yep, I believe I was trying to make this point during bitfield32
> discussion, failed apparently. So if we have "NLA_BITFIELD" with
> arbitrary size, that sounds good to me.
I guess it could be the same way - just have the content be
u32 value[N];
u32 select[N];
where N = nla_len(attr) / 8
That'd be compatible with NLA_BITFIELD32, and we could basically change
all occurrences of NLA_BITFIELD32 to NLA_BITFIELD, and have NLA_BITFIELD
take something like a "max_bit" for the .len field or something like
that? And an entry in the validation union to point to a "u32 *mask"
instead of the current validation_data that just points to a single u32
mask...
So overall seems like a pretty simple extension to NLA_BITFIELD32 that
handles NLA_BITFIELD32 as a special case with simply .len=32.
(len is a 16-bit field, but a 64k bitmap should be sufficient I hope?)
johannes
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Ilias Apalodimas @ 2019-07-04 12:04 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Jose Abreu, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai
In-Reply-To: <20190704135414.0dd5df76@carbon>
Hi Jesper,
> On Thu, 4 Jul 2019 10:13:37 +0000
> Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> > > The page_pool DMA mapping cannot be "kept" when page traveling into the
> > > network stack attached to an SKB. (Ilias and I have a long term plan[1]
> > > to allow this, but you cannot do it ATM).
> >
> > The reason I recycle the page is this previous call to:
> >
> > skb_copy_to_linear_data()
> >
> > So, technically, I'm syncing to CPU the page(s) and then memcpy to a
> > previously allocated SKB ... So it's safe to just recycle the mapping I
> > think.
>
> I didn't notice the skb_copy_to_linear_data(), will copy the entire
> frame, thus leaving the page unused and avail for recycle.
Yea this is essentially a 'copybreak' without the byte limitation that other
drivers usually impose (remember mvneta was doing this for all packets < 256b)
That's why i was concerned on what will happen on > 1000b frames and what the
memory pressure is going to be.
The trade off here is copying vs mapping/unmapping.
>
> Then it looks like you are doing the correct thing. I will appreciate
> if you could add a comment above the call like:
>
> /* Data payload copied into SKB, page ready for recycle */
> page_pool_recycle_direct(rx_q->page_pool, buf->page);
>
>
> > Its kind of using bounce buffers and I do see performance gain in this
> > (I think the reason is because my setup uses swiotlb for DMA mapping).
> >
> > Anyway, I'm open to some suggestions on how to improve this ...
>
> I was surprised to see page_pool being used outside the surrounding XDP
> APIs (included/net/xdp.h). For you use-case, where you "just" use
> page_pool as a driver-local fast recycle-allocator for RX-ring that
> keeps pages DMA mapped, it does make a lot of sense. It simplifies the
> driver a fair amount:
>
> 3 files changed, 63 insertions(+), 144 deletions(-)
>
> Thanks for demonstrating a use-case for page_pool besides XDP, and for
> simplifying a driver with this.
Same here thanks Jose,
>
>
> > > Also remember that the page_pool requires you driver to do the
> > > DMA-sync operation. I see a dma_sync_single_for_cpu(), but I
> > > didn't see a dma_sync_single_for_device() (well, I noticed one
> > > getting removed). (For some HW Ilias tells me that the
> > > dma_sync_single_for_device can be elided, so maybe this can still
> > > be correct for you).
> >
> > My HW just needs descriptors refilled which are in different coherent
> > region so I don't see any reason for dma_sync_single_for_device() ...
>
> For you use-case, given you are copying out the data, and not writing
> into it, then I don't think you need to do sync for device (before
> giving the device the page again for another RX-ring cycle).
>
> The way I understand the danger: if writing to the DMA memory region,
> and not doing the DMA-sync for-device, then the HW/coherency-system can
> write-back the memory later. Which creates a race with the DMA-device,
> if it is receiving a packet and is doing a write into same DMA memory
> region. Someone correct me if I misunderstood this...
Similar understanding here
Cheers
/Ilias
^ permalink raw reply
* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Johannes Berg @ 2019-07-04 12:03 UTC (permalink / raw)
To: Michal Kubecek, netdev
Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, linux-kernel
In-Reply-To: <20190704115236.GR20101@unicorn.suse.cz>
On Thu, 2019-07-04 at 13:52 +0200, Michal Kubecek wrote:
>
> There is still the question if it it should be implemented as a nested
> attribute which could look like the current compact form without the
> "list" flag (if there is no mask, it's a list). Or an unstructured data
> block consisting of u32 bit length
You wouldn't really need the length, since the attribute has a length
already :-)
And then, if you just concatenate the value and mask, the existing
NLA_BITFIELD32 becomes a special case.
> and one or two bitmaps of
> corresponding length. I would prefer the nested attribute, netlink was
> designed to represent structured data, passing structures as binary goes
> against the design (just looked at VFINFO in rtnetlink few days ago,
> it's awful, IMHO).
Yeah, I dunno. On the one hand I completely agree, on the other hand
NLA_BITFIELD32 already goes the other way, and is there now...
> Either way, I would still prefer to have bitmaps represented as an array
> of 32-bit blocks in host byte order. This would be easy to handle in
> kernel both in places where we have u32 based bitmaps and unsigned long
> based ones. Other options seem less appealing:
>
> - u8 based: only complicates processing
> - u64 based: have to care about alignment
> - unsigned long based: alignment and also problems with 64-bit kernel
> vs. 32-bit userspace
Agree with this.
johannes
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jesper Dangaard Brouer @ 2019-07-04 11:54 UTC (permalink / raw)
To: Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai, Ilias Apalodimas,
brouer
In-Reply-To: <BN8PR12MB3266BC5322AADFAC49D9BAFAD3FA0@BN8PR12MB3266.namprd12.prod.outlook.com>
On Thu, 4 Jul 2019 10:13:37 +0000
Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
>
> > The page_pool DMA mapping cannot be "kept" when page traveling into the
> > network stack attached to an SKB. (Ilias and I have a long term plan[1]
> > to allow this, but you cannot do it ATM).
>
> The reason I recycle the page is this previous call to:
>
> skb_copy_to_linear_data()
>
> So, technically, I'm syncing to CPU the page(s) and then memcpy to a
> previously allocated SKB ... So it's safe to just recycle the mapping I
> think.
I didn't notice the skb_copy_to_linear_data(), will copy the entire
frame, thus leaving the page unused and avail for recycle.
Then it looks like you are doing the correct thing. I will appreciate
if you could add a comment above the call like:
/* Data payload copied into SKB, page ready for recycle */
page_pool_recycle_direct(rx_q->page_pool, buf->page);
> Its kind of using bounce buffers and I do see performance gain in this
> (I think the reason is because my setup uses swiotlb for DMA mapping).
>
> Anyway, I'm open to some suggestions on how to improve this ...
I was surprised to see page_pool being used outside the surrounding XDP
APIs (included/net/xdp.h). For you use-case, where you "just" use
page_pool as a driver-local fast recycle-allocator for RX-ring that
keeps pages DMA mapped, it does make a lot of sense. It simplifies the
driver a fair amount:
3 files changed, 63 insertions(+), 144 deletions(-)
Thanks for demonstrating a use-case for page_pool besides XDP, and for
simplifying a driver with this.
> > Also remember that the page_pool requires you driver to do the
> > DMA-sync operation. I see a dma_sync_single_for_cpu(), but I
> > didn't see a dma_sync_single_for_device() (well, I noticed one
> > getting removed). (For some HW Ilias tells me that the
> > dma_sync_single_for_device can be elided, so maybe this can still
> > be correct for you).
>
> My HW just needs descriptors refilled which are in different coherent
> region so I don't see any reason for dma_sync_single_for_device() ...
For you use-case, given you are copying out the data, and not writing
into it, then I don't think you need to do sync for device (before
giving the device the page again for another RX-ring cycle).
The way I understand the danger: if writing to the DMA memory region,
and not doing the DMA-sync for-device, then the HW/coherency-system can
write-back the memory later. Which creates a race with the DMA-device,
if it is receiving a packet and is doing a write into same DMA memory
region. Someone correct me if I misunderstood this...
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net-next v3 1/4] net/sched: Introduce action ct
From: Paul Blakey @ 2019-07-04 11:53 UTC (permalink / raw)
To: Jiri Pirko, Paul Blakey, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, netdev, David Miller, Aaron Conole,
Zhike Wang
Cc: Rony Efraim, nst-kernel, John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <1562241233-5176-1-git-send-email-paulb@mellanox.com>
Allow sending a packet to conntrack module for connection tracking.
The packet will be marked with conntrack connection's state, and
any metadata such as conntrack mark and label. This state metadata
can later be matched against with tc classifers, for example with the
flower classifier as below.
In addition to committing new connections the user can optionally
specific a zone to track within, set a mark/label and configure nat
with an address range and port range.
Usage is as follows:
$ tc qdisc add dev ens1f0_0 ingress
$ tc qdisc add dev ens1f0_1 ingress
$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 0 proto ip \
flower ip_proto tcp ct_state -trk \
action ct zone 2 pipe \
action goto chain 2
$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 2 proto ip \
flower ct_state +trk+new \
action ct zone 2 commit mark 0xbb nat src addr 5.5.5.7 pipe \
action mirred egress redirect dev ens1f0_1
$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 2 proto ip \
flower ct_zone 2 ct_mark 0xbb ct_state +trk+est \
action ct nat pipe \
action mirred egress redirect dev ens1f0_1
$ tc filter add dev ens1f0_1 ingress \
prio 1 chain 0 proto ip \
flower ip_proto tcp ct_state -trk \
action ct zone 2 pipe \
action goto chain 1
$ tc filter add dev ens1f0_1 ingress \
prio 1 chain 1 proto ip \
flower ct_zone 2 ct_mark 0xbb ct_state +trk+est \
action ct nat pipe \
action mirred egress redirect dev ens1f0_0
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Yossi Kuperman <yossiku@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
Changelog:
V2->V3:
Fixed david's comments: Removed extra newline after rcu in tcf_ct_params , and indent of break in act_ct.c
V1->V2:
Fixed parsing of ranges TCA_CT_NAT_IPV6_MAX as 'else' case overwritten ipv4 max
Refactored NAT_PORT_MIN_MAX range handling as well
Added ipv4/ipv6 defragmentation
Removed extra skb pull push of nw offset in exectute nat
Refactored tcf_ct_skb_network_trim after pull
Removed TCA_ACT_CT define
---
include/net/flow_offload.h | 5 +
include/net/tc_act/tc_ct.h | 64 +++
include/uapi/linux/pkt_cls.h | 1 +
include/uapi/linux/tc_act/tc_ct.h | 41 ++
net/sched/Kconfig | 11 +
net/sched/Makefile | 1 +
net/sched/act_ct.c | 978 ++++++++++++++++++++++++++++++++++++++
net/sched/cls_api.c | 5 +
8 files changed, 1106 insertions(+)
create mode 100644 include/net/tc_act/tc_ct.h
create mode 100644 include/uapi/linux/tc_act/tc_ct.h
create mode 100644 net/sched/act_ct.c
diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 36fdb85..5b2c4fa 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -123,6 +123,7 @@ enum flow_action_id {
FLOW_ACTION_QUEUE,
FLOW_ACTION_SAMPLE,
FLOW_ACTION_POLICE,
+ FLOW_ACTION_CT,
};
/* This is mirroring enum pedit_header_type definition for easy mapping between
@@ -172,6 +173,10 @@ struct flow_action_entry {
s64 burst;
u64 rate_bytes_ps;
} police;
+ struct { /* FLOW_ACTION_CT */
+ int action;
+ u16 zone;
+ } ct;
};
};
diff --git a/include/net/tc_act/tc_ct.h b/include/net/tc_act/tc_ct.h
new file mode 100644
index 0000000..59e4f5e
--- /dev/null
+++ b/include/net/tc_act/tc_ct.h
@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __NET_TC_CT_H
+#define __NET_TC_CT_H
+
+#include <net/act_api.h>
+#include <uapi/linux/tc_act/tc_ct.h>
+
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+#include <net/netfilter/nf_nat.h>
+#include <net/netfilter/nf_conntrack_labels.h>
+
+struct tcf_ct_params {
+ struct nf_conn *tmpl;
+ u16 zone;
+
+ u32 mark;
+ u32 mark_mask;
+
+ u32 labels[NF_CT_LABELS_MAX_SIZE / sizeof(u32)];
+ u32 labels_mask[NF_CT_LABELS_MAX_SIZE / sizeof(u32)];
+
+ struct nf_nat_range2 range;
+ bool ipv4_range;
+
+ u16 ct_action;
+
+ struct rcu_head rcu;
+
+};
+
+struct tcf_ct {
+ struct tc_action common;
+ struct tcf_ct_params __rcu *params;
+};
+
+#define to_ct(a) ((struct tcf_ct *)a)
+#define to_ct_params(a) ((struct tcf_ct_params *) \
+ rtnl_dereference((to_ct(a)->params)))
+
+static inline uint16_t tcf_ct_zone(const struct tc_action *a)
+{
+ return to_ct_params(a)->zone;
+}
+
+static inline int tcf_ct_action(const struct tc_action *a)
+{
+ return to_ct_params(a)->ct_action;
+}
+
+#else
+static inline uint16_t tcf_ct_zone(const struct tc_action *a) { return 0; }
+static inline int tcf_ct_action(const struct tc_action *a) { return 0; }
+#endif /* CONFIG_NF_CONNTRACK */
+
+static inline bool is_tcf_ct(const struct tc_action *a)
+{
+#if defined(CONFIG_NET_CLS_ACT) && IS_ENABLED(CONFIG_NF_CONNTRACK)
+ if (a->ops && a->ops->id == TCA_ID_CT)
+ return true;
+#endif
+ return false;
+}
+
+#endif /* __NET_TC_CT_H */
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 8cc6b67..7c9d701 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -106,6 +106,7 @@ enum tca_id {
TCA_ID_SAMPLE = TCA_ACT_SAMPLE,
/* other actions go here */
TCA_ID_CTINFO,
+ TCA_ID_CT,
__TCA_ID_MAX = 255
};
diff --git a/include/uapi/linux/tc_act/tc_ct.h b/include/uapi/linux/tc_act/tc_ct.h
new file mode 100644
index 0000000..5fb1d7a
--- /dev/null
+++ b/include/uapi/linux/tc_act/tc_ct.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef __UAPI_TC_CT_H
+#define __UAPI_TC_CT_H
+
+#include <linux/types.h>
+#include <linux/pkt_cls.h>
+
+enum {
+ TCA_CT_UNSPEC,
+ TCA_CT_PARMS,
+ TCA_CT_TM,
+ TCA_CT_ACTION, /* u16 */
+ TCA_CT_ZONE, /* u16 */
+ TCA_CT_MARK, /* u32 */
+ TCA_CT_MARK_MASK, /* u32 */
+ TCA_CT_LABELS, /* u128 */
+ TCA_CT_LABELS_MASK, /* u128 */
+ TCA_CT_NAT_IPV4_MIN, /* be32 */
+ TCA_CT_NAT_IPV4_MAX, /* be32 */
+ TCA_CT_NAT_IPV6_MIN, /* struct in6_addr */
+ TCA_CT_NAT_IPV6_MAX, /* struct in6_addr */
+ TCA_CT_NAT_PORT_MIN, /* be16 */
+ TCA_CT_NAT_PORT_MAX, /* be16 */
+ TCA_CT_PAD,
+ __TCA_CT_MAX
+};
+
+#define TCA_CT_MAX (__TCA_CT_MAX - 1)
+
+#define TCA_CT_ACT_COMMIT (1 << 0)
+#define TCA_CT_ACT_FORCE (1 << 1)
+#define TCA_CT_ACT_CLEAR (1 << 2)
+#define TCA_CT_ACT_NAT (1 << 3)
+#define TCA_CT_ACT_NAT_SRC (1 << 4)
+#define TCA_CT_ACT_NAT_DST (1 << 5)
+
+struct tc_ct {
+ tc_gen;
+};
+
+#endif /* __UAPI_TC_CT_H */
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 360fdd3..ff90a71 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -929,6 +929,17 @@ config NET_ACT_TUNNEL_KEY
To compile this code as a module, choose M here: the
module will be called act_tunnel_key.
+config NET_ACT_CT
+ tristate "connection tracking tc action"
+ depends on NET_CLS_ACT && NF_CONNTRACK
+ help
+ Say Y here to allow sending the packets to conntrack module.
+
+ If unsure, say N.
+
+ To compile this code as a module, choose M here: the
+ module will be called act_ct.
+
config NET_IFE_SKBMARK
tristate "Support to encoding decoding skb mark on IFE action"
depends on NET_ACT_IFE
diff --git a/net/sched/Makefile b/net/sched/Makefile
index d54bfcb..23d2202 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_NET_IFE_SKBMARK) += act_meta_mark.o
obj-$(CONFIG_NET_IFE_SKBPRIO) += act_meta_skbprio.o
obj-$(CONFIG_NET_IFE_SKBTCINDEX) += act_meta_skbtcindex.o
obj-$(CONFIG_NET_ACT_TUNNEL_KEY)+= act_tunnel_key.o
+obj-$(CONFIG_NET_ACT_CT) += act_ct.o
obj-$(CONFIG_NET_SCH_FIFO) += sch_fifo.o
obj-$(CONFIG_NET_SCH_CBQ) += sch_cbq.o
obj-$(CONFIG_NET_SCH_HTB) += sch_htb.o
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
new file mode 100644
index 0000000..35f7428
--- /dev/null
+++ b/net/sched/act_ct.c
@@ -0,0 +1,978 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/* -
+ * net/sched/act_ct.c Connection Tracking action
+ *
+ * Authors: Paul Blakey <paulb@mellanox.com>
+ * Yossi Kuperman <yossiku@mellanox.com>
+ * Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+#include <linux/rtnetlink.h>
+#include <linux/pkt_cls.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
+#include <net/act_api.h>
+#include <net/ip.h>
+#include <net/ipv6_frag.h>
+#include <uapi/linux/tc_act/tc_ct.h>
+#include <net/tc_act/tc_ct.h>
+
+#include <linux/netfilter/nf_nat.h>
+#include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_core.h>
+#include <net/netfilter/nf_conntrack_zones.h>
+#include <net/netfilter/nf_conntrack_helper.h>
+#include <net/netfilter/ipv6/nf_defrag_ipv6.h>
+
+static struct tc_action_ops act_ct_ops;
+static unsigned int ct_net_id;
+
+struct tc_ct_action_net {
+ struct tc_action_net tn; /* Must be first */
+ bool labels;
+};
+
+/* Determine whether skb->_nfct is equal to the result of conntrack lookup. */
+static bool tcf_ct_skb_nfct_cached(struct net *net, struct sk_buff *skb,
+ u16 zone_id, bool force)
+{
+ enum ip_conntrack_info ctinfo;
+ struct nf_conn *ct;
+
+ ct = nf_ct_get(skb, &ctinfo);
+ if (!ct)
+ return false;
+ if (!net_eq(net, read_pnet(&ct->ct_net)))
+ return false;
+ if (nf_ct_zone(ct)->id != zone_id)
+ return false;
+
+ /* Force conntrack entry direction. */
+ if (force && CTINFO2DIR(ctinfo) != IP_CT_DIR_ORIGINAL) {
+ nf_conntrack_put(&ct->ct_general);
+ nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
+
+ if (nf_ct_is_confirmed(ct))
+ nf_ct_kill(ct);
+
+ return false;
+ }
+
+ return true;
+}
+
+/* Trim the skb to the length specified by the IP/IPv6 header,
+ * removing any trailing lower-layer padding. This prepares the skb
+ * for higher-layer processing that assumes skb->len excludes padding
+ * (such as nf_ip_checksum). The caller needs to pull the skb to the
+ * network header, and ensure ip_hdr/ipv6_hdr points to valid data.
+ */
+static int tcf_ct_skb_network_trim(struct sk_buff *skb, int family)
+{
+ unsigned int len;
+ int err;
+
+ switch (family) {
+ case NFPROTO_IPV4:
+ len = ntohs(ip_hdr(skb)->tot_len);
+ break;
+ case NFPROTO_IPV6:
+ len = sizeof(struct ipv6hdr)
+ + ntohs(ipv6_hdr(skb)->payload_len);
+ break;
+ default:
+ len = skb->len;
+ }
+
+ err = pskb_trim_rcsum(skb, len);
+
+ return err;
+}
+
+static u8 tcf_ct_skb_nf_family(struct sk_buff *skb)
+{
+ u8 family = NFPROTO_UNSPEC;
+
+ switch (skb->protocol) {
+ case htons(ETH_P_IP):
+ family = NFPROTO_IPV4;
+ break;
+ case htons(ETH_P_IPV6):
+ family = NFPROTO_IPV6;
+ break;
+ default:
+ break;
+ }
+
+ return family;
+}
+
+static int tcf_ct_ipv4_is_fragment(struct sk_buff *skb, bool *frag)
+{
+ unsigned int len;
+
+ len = skb_network_offset(skb) + sizeof(struct iphdr);
+ if (unlikely(skb->len < len))
+ return -EINVAL;
+ if (unlikely(!pskb_may_pull(skb, len)))
+ return -ENOMEM;
+
+ *frag = ip_is_fragment(ip_hdr(skb));
+ return 0;
+}
+
+static int tcf_ct_ipv6_is_fragment(struct sk_buff *skb, bool *frag)
+{
+ unsigned int flags = 0, len, payload_ofs = 0;
+ unsigned short frag_off;
+ int nexthdr;
+
+ len = skb_network_offset(skb) + sizeof(struct ipv6hdr);
+ if (unlikely(skb->len < len))
+ return -EINVAL;
+ if (unlikely(!pskb_may_pull(skb, len)))
+ return -ENOMEM;
+
+ nexthdr = ipv6_find_hdr(skb, &payload_ofs, -1, &frag_off, &flags);
+ if (unlikely(nexthdr < 0))
+ return -EPROTO;
+
+ *frag = flags & IP6_FH_F_FRAG;
+ return 0;
+}
+
+static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
+ u8 family, u16 zone)
+{
+ enum ip_conntrack_info ctinfo;
+ struct nf_conn *ct;
+ int err = 0;
+ bool frag;
+
+ /* Previously seen (loopback)? Ignore. */
+ ct = nf_ct_get(skb, &ctinfo);
+ if ((ct && !nf_ct_is_template(ct)) || ctinfo == IP_CT_UNTRACKED)
+ return 0;
+
+ if (family == NFPROTO_IPV4)
+ err = tcf_ct_ipv4_is_fragment(skb, &frag);
+ else
+ err = tcf_ct_ipv6_is_fragment(skb, &frag);
+ if (err || !frag)
+ return err;
+
+ skb_get(skb);
+
+ if (family == NFPROTO_IPV4) {
+ enum ip_defrag_users user = IP_DEFRAG_CONNTRACK_IN + zone;
+
+ memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
+ local_bh_disable();
+ err = ip_defrag(net, skb, user);
+ local_bh_enable();
+ if (err && err != -EINPROGRESS)
+ goto out_free;
+ } else { /* NFPROTO_IPV6 */
+ enum ip6_defrag_users user = IP6_DEFRAG_CONNTRACK_IN + zone;
+
+ memset(IP6CB(skb), 0, sizeof(struct inet6_skb_parm));
+ err = nf_ct_frag6_gather(net, skb, user);
+ if (err && err != -EINPROGRESS)
+ goto out_free;
+ }
+
+ skb_clear_hash(skb);
+ skb->ignore_df = 1;
+ return err;
+
+out_free:
+ kfree_skb(skb);
+ return err;
+}
+
+static void tcf_ct_params_free(struct rcu_head *head)
+{
+ struct tcf_ct_params *params = container_of(head,
+ struct tcf_ct_params, rcu);
+
+ if (params->tmpl)
+ nf_conntrack_put(¶ms->tmpl->ct_general);
+ kfree(params);
+}
+
+#if IS_ENABLED(CONFIG_NF_NAT)
+/* Modelled after nf_nat_ipv[46]_fn().
+ * range is only used for new, uninitialized NAT state.
+ * Returns either NF_ACCEPT or NF_DROP.
+ */
+static int ct_nat_execute(struct sk_buff *skb, struct nf_conn *ct,
+ enum ip_conntrack_info ctinfo,
+ const struct nf_nat_range2 *range,
+ enum nf_nat_manip_type maniptype)
+{
+ int hooknum, err = NF_ACCEPT;
+
+ /* See HOOK2MANIP(). */
+ if (maniptype == NF_NAT_MANIP_SRC)
+ hooknum = NF_INET_LOCAL_IN; /* Source NAT */
+ else
+ hooknum = NF_INET_LOCAL_OUT; /* Destination NAT */
+
+ switch (ctinfo) {
+ case IP_CT_RELATED:
+ case IP_CT_RELATED_REPLY:
+ if (skb->protocol == htons(ETH_P_IP) &&
+ ip_hdr(skb)->protocol == IPPROTO_ICMP) {
+ if (!nf_nat_icmp_reply_translation(skb, ct, ctinfo,
+ hooknum))
+ err = NF_DROP;
+ goto out;
+ } else if (IS_ENABLED(CONFIG_IPV6) &&
+ skb->protocol == htons(ETH_P_IPV6)) {
+ __be16 frag_off;
+ u8 nexthdr = ipv6_hdr(skb)->nexthdr;
+ int hdrlen = ipv6_skip_exthdr(skb,
+ sizeof(struct ipv6hdr),
+ &nexthdr, &frag_off);
+
+ if (hdrlen >= 0 && nexthdr == IPPROTO_ICMPV6) {
+ if (!nf_nat_icmpv6_reply_translation(skb, ct,
+ ctinfo,
+ hooknum,
+ hdrlen))
+ err = NF_DROP;
+ goto out;
+ }
+ }
+ /* Non-ICMP, fall thru to initialize if needed. */
+ /* fall through */
+ case IP_CT_NEW:
+ /* Seen it before? This can happen for loopback, retrans,
+ * or local packets.
+ */
+ if (!nf_nat_initialized(ct, maniptype)) {
+ /* Initialize according to the NAT action. */
+ err = (range && range->flags & NF_NAT_RANGE_MAP_IPS)
+ /* Action is set up to establish a new
+ * mapping.
+ */
+ ? nf_nat_setup_info(ct, range, maniptype)
+ : nf_nat_alloc_null_binding(ct, hooknum);
+ if (err != NF_ACCEPT)
+ goto out;
+ }
+ break;
+
+ case IP_CT_ESTABLISHED:
+ case IP_CT_ESTABLISHED_REPLY:
+ break;
+
+ default:
+ err = NF_DROP;
+ goto out;
+ }
+
+ err = nf_nat_packet(ct, ctinfo, hooknum, skb);
+out:
+ return err;
+}
+#endif /* CONFIG_NF_NAT */
+
+static void tcf_ct_act_set_mark(struct nf_conn *ct, u32 mark, u32 mask)
+{
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
+ u32 new_mark;
+
+ if (!mask)
+ return;
+
+ new_mark = mark | (ct->mark & ~(mask));
+ if (ct->mark != new_mark) {
+ ct->mark = new_mark;
+ if (nf_ct_is_confirmed(ct))
+ nf_conntrack_event_cache(IPCT_MARK, ct);
+ }
+#endif
+}
+
+static void tcf_ct_act_set_labels(struct nf_conn *ct,
+ u32 *labels,
+ u32 *labels_m)
+{
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS)
+ size_t labels_sz = FIELD_SIZEOF(struct tcf_ct_params, labels);
+
+ if (!memchr_inv(labels_m, 0, labels_sz))
+ return;
+
+ nf_connlabels_replace(ct, labels, labels_m, 4);
+#endif
+}
+
+static int tcf_ct_act_nat(struct sk_buff *skb,
+ struct nf_conn *ct,
+ enum ip_conntrack_info ctinfo,
+ int ct_action,
+ struct nf_nat_range2 *range,
+ bool commit)
+{
+#if IS_ENABLED(CONFIG_NF_NAT)
+ enum nf_nat_manip_type maniptype;
+
+ if (!(ct_action & TCA_CT_ACT_NAT))
+ return NF_ACCEPT;
+
+ /* Add NAT extension if not confirmed yet. */
+ if (!nf_ct_is_confirmed(ct) && !nf_ct_nat_ext_add(ct))
+ return NF_DROP; /* Can't NAT. */
+
+ if (ctinfo != IP_CT_NEW && (ct->status & IPS_NAT_MASK) &&
+ (ctinfo != IP_CT_RELATED || commit)) {
+ /* NAT an established or related connection like before. */
+ if (CTINFO2DIR(ctinfo) == IP_CT_DIR_REPLY)
+ /* This is the REPLY direction for a connection
+ * for which NAT was applied in the forward
+ * direction. Do the reverse NAT.
+ */
+ maniptype = ct->status & IPS_SRC_NAT
+ ? NF_NAT_MANIP_DST : NF_NAT_MANIP_SRC;
+ else
+ maniptype = ct->status & IPS_SRC_NAT
+ ? NF_NAT_MANIP_SRC : NF_NAT_MANIP_DST;
+ } else if (ct_action & TCA_CT_ACT_NAT_SRC) {
+ maniptype = NF_NAT_MANIP_SRC;
+ } else if (ct_action & TCA_CT_ACT_NAT_DST) {
+ maniptype = NF_NAT_MANIP_DST;
+ } else {
+ return NF_ACCEPT;
+ }
+
+ return ct_nat_execute(skb, ct, ctinfo, range, maniptype);
+#else
+ return NF_ACCEPT;
+#endif
+}
+
+static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
+ struct tcf_result *res)
+{
+ struct net *net = dev_net(skb->dev);
+ bool cached, commit, clear, force;
+ enum ip_conntrack_info ctinfo;
+ struct tcf_ct *c = to_ct(a);
+ struct nf_conn *tmpl = NULL;
+ struct nf_hook_state state;
+ int nh_ofs, err, retval;
+ struct tcf_ct_params *p;
+ struct nf_conn *ct;
+ u8 family;
+
+ p = rcu_dereference_bh(c->params);
+
+ retval = READ_ONCE(c->tcf_action);
+ commit = p->ct_action & TCA_CT_ACT_COMMIT;
+ clear = p->ct_action & TCA_CT_ACT_CLEAR;
+ force = p->ct_action & TCA_CT_ACT_FORCE;
+ tmpl = p->tmpl;
+
+ if (clear) {
+ ct = nf_ct_get(skb, &ctinfo);
+ if (ct) {
+ nf_conntrack_put(&ct->ct_general);
+ nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
+ }
+
+ goto out;
+ }
+
+ family = tcf_ct_skb_nf_family(skb);
+ if (family == NFPROTO_UNSPEC)
+ goto drop;
+
+ /* The conntrack module expects to be working at L3.
+ * We also try to pull the IPv4/6 header to linear area
+ */
+ nh_ofs = skb_network_offset(skb);
+ skb_pull_rcsum(skb, nh_ofs);
+ err = tcf_ct_handle_fragments(net, skb, family, p->zone);
+ if (err == -EINPROGRESS) {
+ retval = TC_ACT_STOLEN;
+ goto out;
+ }
+ if (err)
+ goto drop;
+
+ err = tcf_ct_skb_network_trim(skb, family);
+ if (err)
+ goto drop;
+
+ /* If we are recirculating packets to match on ct fields and
+ * committing with a separate ct action, then we don't need to
+ * actually run the packet through conntrack twice unless it's for a
+ * different zone.
+ */
+ cached = tcf_ct_skb_nfct_cached(net, skb, p->zone, force);
+ if (!cached) {
+ /* Associate skb with specified zone. */
+ if (tmpl) {
+ ct = nf_ct_get(skb, &ctinfo);
+ if (skb_nfct(skb))
+ nf_conntrack_put(skb_nfct(skb));
+ nf_conntrack_get(&tmpl->ct_general);
+ nf_ct_set(skb, tmpl, IP_CT_NEW);
+ }
+
+ state.hook = NF_INET_PRE_ROUTING;
+ state.net = net;
+ state.pf = family;
+ err = nf_conntrack_in(skb, &state);
+ if (err != NF_ACCEPT)
+ goto out_push;
+ }
+
+ ct = nf_ct_get(skb, &ctinfo);
+ if (!ct)
+ goto out_push;
+ nf_ct_deliver_cached_events(ct);
+
+ err = tcf_ct_act_nat(skb, ct, ctinfo, p->ct_action, &p->range, commit);
+ if (err != NF_ACCEPT)
+ goto drop;
+
+ if (commit) {
+ tcf_ct_act_set_mark(ct, p->mark, p->mark_mask);
+ tcf_ct_act_set_labels(ct, p->labels, p->labels_mask);
+
+ /* This will take care of sending queued events
+ * even if the connection is already confirmed.
+ */
+ nf_conntrack_confirm(skb);
+ }
+
+out_push:
+ skb_push_rcsum(skb, nh_ofs);
+
+out:
+ bstats_cpu_update(this_cpu_ptr(a->cpu_bstats), skb);
+ return retval;
+
+drop:
+ qstats_drop_inc(this_cpu_ptr(a->cpu_qstats));
+ return TC_ACT_SHOT;
+}
+
+static const struct nla_policy ct_policy[TCA_CT_MAX + 1] = {
+ [TCA_CT_ACTION] = { .type = NLA_U16 },
+ [TCA_CT_PARMS] = { .type = NLA_EXACT_LEN, .len = sizeof(struct tc_ct) },
+ [TCA_CT_ZONE] = { .type = NLA_U16 },
+ [TCA_CT_MARK] = { .type = NLA_U32 },
+ [TCA_CT_MARK_MASK] = { .type = NLA_U32 },
+ [TCA_CT_LABELS] = { .type = NLA_BINARY,
+ .len = 128 / BITS_PER_BYTE },
+ [TCA_CT_LABELS_MASK] = { .type = NLA_BINARY,
+ .len = 128 / BITS_PER_BYTE },
+ [TCA_CT_NAT_IPV4_MIN] = { .type = NLA_U32 },
+ [TCA_CT_NAT_IPV4_MAX] = { .type = NLA_U32 },
+ [TCA_CT_NAT_IPV6_MIN] = { .type = NLA_EXACT_LEN,
+ .len = sizeof(struct in6_addr) },
+ [TCA_CT_NAT_IPV6_MAX] = { .type = NLA_EXACT_LEN,
+ .len = sizeof(struct in6_addr) },
+ [TCA_CT_NAT_PORT_MIN] = { .type = NLA_U16 },
+ [TCA_CT_NAT_PORT_MAX] = { .type = NLA_U16 },
+};
+
+static int tcf_ct_fill_params_nat(struct tcf_ct_params *p,
+ struct tc_ct *parm,
+ struct nlattr **tb,
+ struct netlink_ext_ack *extack)
+{
+ struct nf_nat_range2 *range;
+
+ if (!(p->ct_action & TCA_CT_ACT_NAT))
+ return 0;
+
+ if (!IS_ENABLED(CONFIG_NF_NAT)) {
+ NL_SET_ERR_MSG_MOD(extack, "Netfilter nat isn't enabled in kernel");
+ return -EOPNOTSUPP;
+ }
+
+ if (!(p->ct_action & (TCA_CT_ACT_NAT_SRC | TCA_CT_ACT_NAT_DST)))
+ return 0;
+
+ if ((p->ct_action & TCA_CT_ACT_NAT_SRC) &&
+ (p->ct_action & TCA_CT_ACT_NAT_DST)) {
+ NL_SET_ERR_MSG_MOD(extack, "dnat and snat can't be enabled at the same time");
+ return -EOPNOTSUPP;
+ }
+
+ range = &p->range;
+ if (tb[TCA_CT_NAT_IPV4_MIN]) {
+ struct nlattr *max_attr = tb[TCA_CT_NAT_IPV4_MAX];
+
+ p->ipv4_range = true;
+ range->flags |= NF_NAT_RANGE_MAP_IPS;
+ range->min_addr.ip =
+ nla_get_in_addr(tb[TCA_CT_NAT_IPV4_MIN]);
+
+ range->max_addr.ip = max_attr ?
+ nla_get_in_addr(max_attr) :
+ range->min_addr.ip;
+ } else if (tb[TCA_CT_NAT_IPV6_MIN]) {
+ struct nlattr *max_attr = tb[TCA_CT_NAT_IPV6_MAX];
+
+ p->ipv4_range = false;
+ range->flags |= NF_NAT_RANGE_MAP_IPS;
+ range->min_addr.in6 =
+ nla_get_in6_addr(tb[TCA_CT_NAT_IPV6_MIN]);
+
+ range->max_addr.in6 = max_attr ?
+ nla_get_in6_addr(max_attr) :
+ range->min_addr.in6;
+ }
+
+ if (tb[TCA_CT_NAT_PORT_MIN]) {
+ range->flags |= NF_NAT_RANGE_PROTO_SPECIFIED;
+ range->min_proto.all = nla_get_be16(tb[TCA_CT_NAT_PORT_MIN]);
+
+ range->max_proto.all = tb[TCA_CT_NAT_PORT_MAX] ?
+ nla_get_be16(tb[TCA_CT_NAT_PORT_MAX]) :
+ range->min_proto.all;
+ }
+
+ return 0;
+}
+
+static void tcf_ct_set_key_val(struct nlattr **tb,
+ void *val, int val_type,
+ void *mask, int mask_type,
+ int len)
+{
+ if (!tb[val_type])
+ return;
+ nla_memcpy(val, tb[val_type], len);
+
+ if (!mask)
+ return;
+
+ if (mask_type == TCA_CT_UNSPEC || !tb[mask_type])
+ memset(mask, 0xff, len);
+ else
+ nla_memcpy(mask, tb[mask_type], len);
+}
+
+static int tcf_ct_fill_params(struct net *net,
+ struct tcf_ct_params *p,
+ struct tc_ct *parm,
+ struct nlattr **tb,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_ct_action_net *tn = net_generic(net, ct_net_id);
+ struct nf_conntrack_zone zone;
+ struct nf_conn *tmpl;
+ int err;
+
+ p->zone = NF_CT_DEFAULT_ZONE_ID;
+
+ tcf_ct_set_key_val(tb,
+ &p->ct_action, TCA_CT_ACTION,
+ NULL, TCA_CT_UNSPEC,
+ sizeof(p->ct_action));
+
+ if (p->ct_action & TCA_CT_ACT_CLEAR)
+ return 0;
+
+ err = tcf_ct_fill_params_nat(p, parm, tb, extack);
+ if (err)
+ return err;
+
+ if (tb[TCA_CT_MARK]) {
+ if (!IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)) {
+ NL_SET_ERR_MSG_MOD(extack, "Conntrack mark isn't enabled.");
+ return -EOPNOTSUPP;
+ }
+ tcf_ct_set_key_val(tb,
+ &p->mark, TCA_CT_MARK,
+ &p->mark_mask, TCA_CT_MARK_MASK,
+ sizeof(p->mark));
+ }
+
+ if (tb[TCA_CT_LABELS]) {
+ if (!IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS)) {
+ NL_SET_ERR_MSG_MOD(extack, "Conntrack labels isn't enabled.");
+ return -EOPNOTSUPP;
+ }
+
+ if (!tn->labels) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to set connlabel length");
+ return -EOPNOTSUPP;
+ }
+ tcf_ct_set_key_val(tb,
+ p->labels, TCA_CT_LABELS,
+ p->labels_mask, TCA_CT_LABELS_MASK,
+ sizeof(p->labels));
+ }
+
+ if (tb[TCA_CT_ZONE]) {
+ if (!IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)) {
+ NL_SET_ERR_MSG_MOD(extack, "Conntrack zones isn't enabled.");
+ return -EOPNOTSUPP;
+ }
+
+ tcf_ct_set_key_val(tb,
+ &p->zone, TCA_CT_ZONE,
+ NULL, TCA_CT_UNSPEC,
+ sizeof(p->zone));
+ }
+
+ if (p->zone == NF_CT_DEFAULT_ZONE_ID)
+ return 0;
+
+ nf_ct_zone_init(&zone, p->zone, NF_CT_DEFAULT_ZONE_DIR, 0);
+ tmpl = nf_ct_tmpl_alloc(net, &zone, GFP_KERNEL);
+ if (!tmpl) {
+ NL_SET_ERR_MSG_MOD(extack, "Failed to allocate conntrack template");
+ return -ENOMEM;
+ }
+ __set_bit(IPS_CONFIRMED_BIT, &tmpl->status);
+ nf_conntrack_get(&tmpl->ct_general);
+ p->tmpl = tmpl;
+
+ return 0;
+}
+
+static int tcf_ct_init(struct net *net, struct nlattr *nla,
+ struct nlattr *est, struct tc_action **a,
+ int replace, int bind, bool rtnl_held,
+ struct tcf_proto *tp,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_action_net *tn = net_generic(net, ct_net_id);
+ struct tcf_ct_params *params = NULL;
+ struct nlattr *tb[TCA_CT_MAX + 1];
+ struct tcf_chain *goto_ch = NULL;
+ struct tc_ct *parm;
+ struct tcf_ct *c;
+ int err, res = 0;
+
+ if (!nla) {
+ NL_SET_ERR_MSG_MOD(extack, "Ct requires attributes to be passed");
+ return -EINVAL;
+ }
+
+ err = nla_parse_nested(tb, TCA_CT_MAX, nla, ct_policy, extack);
+ if (err < 0)
+ return err;
+
+ if (!tb[TCA_CT_PARMS]) {
+ NL_SET_ERR_MSG_MOD(extack, "Missing required ct parameters");
+ return -EINVAL;
+ }
+ parm = nla_data(tb[TCA_CT_PARMS]);
+
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+
+ if (!err) {
+ err = tcf_idr_create(tn, parm->index, est, a,
+ &act_ct_ops, bind, true);
+ if (err) {
+ tcf_idr_cleanup(tn, parm->index);
+ return err;
+ }
+ res = ACT_P_CREATED;
+ } else {
+ if (bind)
+ return 0;
+
+ if (!replace) {
+ tcf_idr_release(*a, bind);
+ return -EEXIST;
+ }
+ }
+ err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
+ if (err < 0)
+ goto cleanup;
+
+ c = to_ct(*a);
+
+ params = kzalloc(sizeof(*params), GFP_KERNEL);
+ if (unlikely(!params)) {
+ err = -ENOMEM;
+ goto cleanup;
+ }
+
+ err = tcf_ct_fill_params(net, params, parm, tb, extack);
+ if (err)
+ goto cleanup;
+
+ spin_lock_bh(&c->tcf_lock);
+ goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
+ rcu_swap_protected(c->params, params, lockdep_is_held(&c->tcf_lock));
+ spin_unlock_bh(&c->tcf_lock);
+
+ if (goto_ch)
+ tcf_chain_put_by_act(goto_ch);
+ if (params)
+ kfree_rcu(params, rcu);
+ if (res == ACT_P_CREATED)
+ tcf_idr_insert(tn, *a);
+
+ return res;
+
+cleanup:
+ if (goto_ch)
+ tcf_chain_put_by_act(goto_ch);
+ kfree(params);
+ tcf_idr_release(*a, bind);
+ return err;
+}
+
+static void tcf_ct_cleanup(struct tc_action *a)
+{
+ struct tcf_ct_params *params;
+ struct tcf_ct *c = to_ct(a);
+
+ params = rcu_dereference_protected(c->params, 1);
+ if (params)
+ call_rcu(¶ms->rcu, tcf_ct_params_free);
+}
+
+static int tcf_ct_dump_key_val(struct sk_buff *skb,
+ void *val, int val_type,
+ void *mask, int mask_type,
+ int len)
+{
+ int err;
+
+ if (mask && !memchr_inv(mask, 0, len))
+ return 0;
+
+ err = nla_put(skb, val_type, len, val);
+ if (err)
+ return err;
+
+ if (mask_type != TCA_CT_UNSPEC) {
+ err = nla_put(skb, mask_type, len, mask);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static int tcf_ct_dump_nat(struct sk_buff *skb, struct tcf_ct_params *p)
+{
+ struct nf_nat_range2 *range = &p->range;
+
+ if (!(p->ct_action & TCA_CT_ACT_NAT))
+ return 0;
+
+ if (!(p->ct_action & (TCA_CT_ACT_NAT_SRC | TCA_CT_ACT_NAT_DST)))
+ return 0;
+
+ if (range->flags & NF_NAT_RANGE_MAP_IPS) {
+ if (p->ipv4_range) {
+ if (nla_put_in_addr(skb, TCA_CT_NAT_IPV4_MIN,
+ range->min_addr.ip))
+ return -1;
+ if (nla_put_in_addr(skb, TCA_CT_NAT_IPV4_MAX,
+ range->max_addr.ip))
+ return -1;
+ } else {
+ if (nla_put_in6_addr(skb, TCA_CT_NAT_IPV6_MIN,
+ &range->min_addr.in6))
+ return -1;
+ if (nla_put_in6_addr(skb, TCA_CT_NAT_IPV6_MAX,
+ &range->max_addr.in6))
+ return -1;
+ }
+ }
+
+ if (range->flags & NF_NAT_RANGE_PROTO_SPECIFIED) {
+ if (nla_put_be16(skb, TCA_CT_NAT_PORT_MIN,
+ range->min_proto.all))
+ return -1;
+ if (nla_put_be16(skb, TCA_CT_NAT_PORT_MAX,
+ range->max_proto.all))
+ return -1;
+ }
+
+ return 0;
+}
+
+static inline int tcf_ct_dump(struct sk_buff *skb, struct tc_action *a,
+ int bind, int ref)
+{
+ unsigned char *b = skb_tail_pointer(skb);
+ struct tcf_ct *c = to_ct(a);
+ struct tcf_ct_params *p;
+
+ struct tc_ct opt = {
+ .index = c->tcf_index,
+ .refcnt = refcount_read(&c->tcf_refcnt) - ref,
+ .bindcnt = atomic_read(&c->tcf_bindcnt) - bind,
+ };
+ struct tcf_t t;
+
+ spin_lock_bh(&c->tcf_lock);
+ p = rcu_dereference_protected(c->params,
+ lockdep_is_held(&c->tcf_lock));
+ opt.action = c->tcf_action;
+
+ if (tcf_ct_dump_key_val(skb,
+ &p->ct_action, TCA_CT_ACTION,
+ NULL, TCA_CT_UNSPEC,
+ sizeof(p->ct_action)))
+ goto nla_put_failure;
+
+ if (p->ct_action & TCA_CT_ACT_CLEAR)
+ goto skip_dump;
+
+ if (IS_ENABLED(CONFIG_NF_CONNTRACK_MARK) &&
+ tcf_ct_dump_key_val(skb,
+ &p->mark, TCA_CT_MARK,
+ &p->mark_mask, TCA_CT_MARK_MASK,
+ sizeof(p->mark)))
+ goto nla_put_failure;
+
+ if (IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS) &&
+ tcf_ct_dump_key_val(skb,
+ p->labels, TCA_CT_LABELS,
+ p->labels_mask, TCA_CT_LABELS_MASK,
+ sizeof(p->labels)))
+ goto nla_put_failure;
+
+ if (IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES) &&
+ tcf_ct_dump_key_val(skb,
+ &p->zone, TCA_CT_ZONE,
+ NULL, TCA_CT_UNSPEC,
+ sizeof(p->zone)))
+ goto nla_put_failure;
+
+ if (tcf_ct_dump_nat(skb, p))
+ goto nla_put_failure;
+
+skip_dump:
+ if (nla_put(skb, TCA_CT_PARMS, sizeof(opt), &opt))
+ goto nla_put_failure;
+
+ tcf_tm_dump(&t, &c->tcf_tm);
+ if (nla_put_64bit(skb, TCA_CT_TM, sizeof(t), &t, TCA_CT_PAD))
+ goto nla_put_failure;
+ spin_unlock_bh(&c->tcf_lock);
+
+ return skb->len;
+nla_put_failure:
+ spin_unlock_bh(&c->tcf_lock);
+ nlmsg_trim(skb, b);
+ return -1;
+}
+
+static int tcf_ct_walker(struct net *net, struct sk_buff *skb,
+ struct netlink_callback *cb, int type,
+ const struct tc_action_ops *ops,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_action_net *tn = net_generic(net, ct_net_id);
+
+ return tcf_generic_walker(tn, skb, cb, type, ops, extack);
+}
+
+static int tcf_ct_search(struct net *net, struct tc_action **a, u32 index)
+{
+ struct tc_action_net *tn = net_generic(net, ct_net_id);
+
+ return tcf_idr_search(tn, a, index);
+}
+
+static void tcf_stats_update(struct tc_action *a, u64 bytes, u32 packets,
+ u64 lastuse, bool hw)
+{
+ struct tcf_ct *c = to_ct(a);
+
+ _bstats_cpu_update(this_cpu_ptr(a->cpu_bstats), bytes, packets);
+
+ if (hw)
+ _bstats_cpu_update(this_cpu_ptr(a->cpu_bstats_hw),
+ bytes, packets);
+ c->tcf_tm.lastuse = max_t(u64, c->tcf_tm.lastuse, lastuse);
+}
+
+static struct tc_action_ops act_ct_ops = {
+ .kind = "ct",
+ .id = TCA_ID_CT,
+ .owner = THIS_MODULE,
+ .act = tcf_ct_act,
+ .dump = tcf_ct_dump,
+ .init = tcf_ct_init,
+ .cleanup = tcf_ct_cleanup,
+ .walk = tcf_ct_walker,
+ .lookup = tcf_ct_search,
+ .stats_update = tcf_stats_update,
+ .size = sizeof(struct tcf_ct),
+};
+
+static __net_init int ct_init_net(struct net *net)
+{
+ struct tc_ct_action_net *tn = net_generic(net, ct_net_id);
+ unsigned int n_bits = FIELD_SIZEOF(struct tcf_ct_params, labels) * 8;
+
+ if (nf_connlabels_get(net, n_bits - 1)) {
+ tn->labels = false;
+ pr_err("act_ct: Failed to set connlabels length");
+ } else {
+ tn->labels = true;
+ }
+
+ return tc_action_net_init(&tn->tn, &act_ct_ops);
+}
+
+static void __net_exit ct_exit_net(struct list_head *net_list)
+{
+ struct net *net;
+
+ rtnl_lock();
+ list_for_each_entry(net, net_list, exit_list) {
+ struct tc_ct_action_net *tn = net_generic(net, ct_net_id);
+
+ if (tn->labels)
+ nf_connlabels_put(net);
+ }
+ rtnl_unlock();
+
+ tc_action_net_exit(net_list, ct_net_id);
+}
+
+static struct pernet_operations ct_net_ops = {
+ .init = ct_init_net,
+ .exit_batch = ct_exit_net,
+ .id = &ct_net_id,
+ .size = sizeof(struct tc_ct_action_net),
+};
+
+static int __init ct_init_module(void)
+{
+ return tcf_register_action(&act_ct_ops, &ct_net_ops);
+}
+
+static void __exit ct_cleanup_module(void)
+{
+ tcf_unregister_action(&act_ct_ops, &ct_net_ops);
+}
+
+module_init(ct_init_module);
+module_exit(ct_cleanup_module);
+MODULE_AUTHOR("Paul Blakey <paulb@mellanox.com>");
+MODULE_AUTHOR("Yossi Kuperman <yossiku@mellanox.com>");
+MODULE_AUTHOR("Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>");
+MODULE_DESCRIPTION("Connection tracking action");
+MODULE_LICENSE("GPL v2");
+
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index ad36bbc..4a7331c 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -35,6 +35,7 @@
#include <net/tc_act/tc_police.h>
#include <net/tc_act/tc_sample.h>
#include <net/tc_act/tc_skbedit.h>
+#include <net/tc_act/tc_ct.h>
extern const struct nla_policy rtm_tca_policy[TCA_MAX + 1];
@@ -3266,6 +3267,10 @@ int tc_setup_flow_action(struct flow_action *flow_action,
entry->police.burst = tcf_police_tcfp_burst(act);
entry->police.rate_bytes_ps =
tcf_police_rate_bytes_ps(act);
+ } else if (is_tcf_ct(act)) {
+ entry->id = FLOW_ACTION_CT;
+ entry->ct.action = tcf_ct_action(act);
+ entry->ct.zone = tcf_ct_zone(act);
} else {
goto err_out;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 4/4] tc-tests: Add tc action ct tests
From: Paul Blakey @ 2019-07-04 11:53 UTC (permalink / raw)
To: Jiri Pirko, Paul Blakey, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, netdev, David Miller, Aaron Conole,
Zhike Wang
Cc: Rony Efraim, nst-kernel, John Hurley, Simon Horman, Justin Pettit,
Marcelo Ricardo Leitner
In-Reply-To: <1562241233-5176-1-git-send-email-paulb@mellanox.com>
Add 13 tests ensuring the command line is doing what is supposed to do.
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
---
.../selftests/tc-testing/tc-tests/actions/ct.json | 314 +++++++++++++++++++++
1 file changed, 314 insertions(+)
create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json b/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
new file mode 100644
index 0000000..62b82fe
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
@@ -0,0 +1,314 @@
+[
+ {
+ "id": "696a",
+ "name": "Add simple ct action",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct index 42",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct zone 0 pipe.*index 42 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "9f20",
+ "name": "Add ct clear action",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct clear index 42",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct clear pipe.*index 42 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "5bea",
+ "name": "Try ct with zone",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct zone 404 index 42",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct zone 404 pipe.*index 42 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "d5d6",
+ "name": "Try ct with zone, commit",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct zone 404 commit index 42",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct commit zone 404 pipe.*index 42 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "029f",
+ "name": "Try ct with zone, commit, mark",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct zone 404 commit mark 0x42 index 42",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct commit mark 66 zone 404 pipe.*index 42 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "a58d",
+ "name": "Try ct with zone, commit, mark, nat",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct zone 404 commit mark 0x42 nat src addr 5.5.5.7 index 42",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct commit mark 66 zone 404 nat src addr 5.5.5.7 pipe.*index 42 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "901b",
+ "name": "Try ct with full nat ipv4 range syntax",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct commit nat src addr 5.5.5.7-5.5.6.0 port 1000-2000 index 44",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct commit zone 0 nat src addr 5.5.5.7-5.5.6.0 port 1000-2000 pipe.*index 44 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "072b",
+ "name": "Try ct with full nat ipv6 syntax",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct commit nat src addr 2001::1 port 1000-2000 index 44",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct commit zone 0 nat src addr 2001::1 port 1000-2000 pipe.*index 44 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "3420",
+ "name": "Try ct with full nat ipv6 range syntax",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct commit nat src addr 2001::1-2001::10 port 1000-2000 index 44",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct commit zone 0 nat src addr 2001::1-2001::10 port 1000-2000 pipe.*index 44 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "4470",
+ "name": "Try ct with full nat ipv6 range syntax + force",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct commit force nat src addr 2001::1-2001::10 port 1000-2000 index 44",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct commit force zone 0 nat src addr 2001::1-2001::10 port 1000-2000 pipe.*index 44 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "5d88",
+ "name": "Try ct with label",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct label 123123 index 44",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct zone 0 label 12312300000000000000000000000000 pipe.*index 44 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "04d4",
+ "name": "Try ct with label with mask",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct label 12312300000000000000000000000001/ffffffff000000000000000000000001 index 44",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct zone 0 label 12312300000000000000000000000001/ffffffff000000000000000000000001 pipe.*index 44 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ },
+ {
+ "id": "9751",
+ "name": "Try ct with mark + mask",
+ "category": [
+ "actions",
+ "ct"
+ ],
+ "setup": [
+ [
+ "$TC actions flush action ct",
+ 0,
+ 1,
+ 255
+ ]
+ ],
+ "cmdUnderTest": "$TC actions add action ct mark 0x42/0xf0 index 42",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action ct",
+ "matchPattern": "action order [0-9]*: ct mark 66/0xf0 zone 0 pipe.*index 42 ref",
+ "matchCount": "1",
+ "teardown": [
+ "$TC actions flush action ct"
+ ]
+ }
+]
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 2/4] net/flow_dissector: add connection tracking dissection
From: Paul Blakey @ 2019-07-04 11:53 UTC (permalink / raw)
To: Jiri Pirko, Paul Blakey, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, netdev, David Miller, Aaron Conole,
Zhike Wang
Cc: Rony Efraim, nst-kernel, John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <1562241233-5176-1-git-send-email-paulb@mellanox.com>
Retreives connection tracking zone, mark, label, and state from
a SKB.
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/linux/skbuff.h | 10 ++++++++++
include/net/flow_dissector.h | 15 +++++++++++++++
net/core/flow_dissector.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 69 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 28bdaf9..e91cc60 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1320,6 +1320,16 @@ static inline bool skb_flow_dissect_flow_keys(const struct sk_buff *skb,
data, proto, nhoff, hlen, flags);
}
+/* Gets a skb connection tracking info, ctinfo map should be a
+ * a map of mapsize to translate enum ip_conntrack_info states
+ * to user states.
+ */
+void
+skb_flow_dissect_ct(const struct sk_buff *skb,
+ struct flow_dissector *flow_dissector,
+ void *target_container,
+ u16 *ctinfo_map,
+ size_t mapsize);
void
skb_flow_dissect_tunnel_info(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index d7ce647..76dcba3 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -200,6 +200,20 @@ struct flow_dissector_key_ip {
__u8 ttl;
};
+/**
+ * struct flow_dissector_key_ct:
+ * @ct_state: conntrack state after converting with map
+ * @ct_mark: conttrack mark
+ * @ct_zone: conntrack zone
+ * @ct_labels: conntrack labels
+ */
+struct flow_dissector_key_ct {
+ u16 ct_state;
+ u16 ct_zone;
+ u32 ct_mark;
+ u32 ct_labels[4];
+};
+
enum flow_dissector_key_id {
FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
@@ -225,6 +239,7 @@ enum flow_dissector_key_id {
FLOW_DISSECTOR_KEY_CVLAN, /* struct flow_dissector_key_vlan */
FLOW_DISSECTOR_KEY_ENC_IP, /* struct flow_dissector_key_ip */
FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */
+ FLOW_DISSECTOR_KEY_CT, /* struct flow_dissector_key_ct */
FLOW_DISSECTOR_KEY_MAX,
};
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index c0559af..293bffc 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -27,6 +27,10 @@
#include <scsi/fc/fc_fcoe.h>
#include <uapi/linux/batadv_packet.h>
#include <linux/bpf.h>
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+#include <net/netfilter/nf_conntrack_core.h>
+#include <net/netfilter/nf_conntrack_labels.h>
+#endif
static DEFINE_MUTEX(flow_dissector_mutex);
@@ -216,6 +220,46 @@ __be32 __skb_flow_get_ports(const struct sk_buff *skb, int thoff, u8 ip_proto,
}
void
+skb_flow_dissect_ct(const struct sk_buff *skb,
+ struct flow_dissector *flow_dissector,
+ void *target_container,
+ u16 *ctinfo_map,
+ size_t mapsize)
+{
+#if IS_ENABLED(CONFIG_NF_CONNTRACK)
+ struct flow_dissector_key_ct *key;
+ enum ip_conntrack_info ctinfo;
+ struct nf_conn_labels *cl;
+ struct nf_conn *ct;
+
+ if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_CT))
+ return;
+
+ ct = nf_ct_get(skb, &ctinfo);
+ if (!ct)
+ return;
+
+ key = skb_flow_dissector_target(flow_dissector,
+ FLOW_DISSECTOR_KEY_CT,
+ target_container);
+
+ if (ctinfo < mapsize)
+ key->ct_state = ctinfo_map[ctinfo];
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)
+ key->ct_zone = ct->zone.id;
+#endif
+#if IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)
+ key->ct_mark = ct->mark;
+#endif
+
+ cl = nf_ct_labels_find(ct);
+ if (cl)
+ memcpy(key->ct_labels, cl->bits, sizeof(key->ct_labels));
+#endif /* CONFIG_NF_CONNTRACK */
+}
+EXPORT_SYMBOL(skb_flow_dissect_ct);
+
+void
skb_flow_dissect_tunnel_info(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
void *target_container)
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 3/4] net/sched: cls_flower: Add matching on conntrack info
From: Paul Blakey @ 2019-07-04 11:53 UTC (permalink / raw)
To: Jiri Pirko, Paul Blakey, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, netdev, David Miller, Aaron Conole,
Zhike Wang
Cc: Rony Efraim, nst-kernel, John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <1562241233-5176-1-git-send-email-paulb@mellanox.com>
New matches for conntrack mark, label, zone, and state.
Signed-off-by: Paul Blakey <paulb@mellanox.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Yossi Kuperman <yossiku@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/uapi/linux/pkt_cls.h | 16 ++++++
net/sched/cls_flower.c | 127 +++++++++++++++++++++++++++++++++++++++++--
2 files changed, 138 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 7c9d701..6992df1 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -536,12 +536,28 @@ enum {
TCA_FLOWER_KEY_PORT_DST_MIN, /* be16 */
TCA_FLOWER_KEY_PORT_DST_MAX, /* be16 */
+ TCA_FLOWER_KEY_CT_STATE, /* u16 */
+ TCA_FLOWER_KEY_CT_STATE_MASK, /* u16 */
+ TCA_FLOWER_KEY_CT_ZONE, /* u16 */
+ TCA_FLOWER_KEY_CT_ZONE_MASK, /* u16 */
+ TCA_FLOWER_KEY_CT_MARK, /* u32 */
+ TCA_FLOWER_KEY_CT_MARK_MASK, /* u32 */
+ TCA_FLOWER_KEY_CT_LABELS, /* u128 */
+ TCA_FLOWER_KEY_CT_LABELS_MASK, /* u128 */
+
__TCA_FLOWER_MAX,
};
#define TCA_FLOWER_MAX (__TCA_FLOWER_MAX - 1)
enum {
+ TCA_FLOWER_KEY_CT_FLAGS_NEW = 1 << 0, /* Beginning of a new connection. */
+ TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED = 1 << 1, /* Part of an existing connection. */
+ TCA_FLOWER_KEY_CT_FLAGS_RELATED = 1 << 2, /* Related to an established connection. */
+ TCA_FLOWER_KEY_CT_FLAGS_TRACKED = 1 << 3, /* Conntrack has occurred. */
+};
+
+enum {
TCA_FLOWER_KEY_ENC_OPTS_UNSPEC,
TCA_FLOWER_KEY_ENC_OPTS_GENEVE, /* Nested
* TCA_FLOWER_KEY_ENC_OPT_GENEVE_
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index bd1767d..883bb3b 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -26,6 +26,8 @@
#include <net/dst.h>
#include <net/dst_metadata.h>
+#include <uapi/linux/netfilter/nf_conntrack_common.h>
+
struct fl_flow_key {
int indev_ifindex;
struct flow_dissector_key_control control;
@@ -54,6 +56,7 @@ struct fl_flow_key {
struct flow_dissector_key_enc_opts enc_opts;
struct flow_dissector_key_ports tp_min;
struct flow_dissector_key_ports tp_max;
+ struct flow_dissector_key_ct ct;
} __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
struct fl_flow_mask_range {
@@ -272,14 +275,27 @@ static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask,
return __fl_lookup(mask, mkey);
}
+static u16 fl_ct_info_to_flower_map[] = {
+ [IP_CT_ESTABLISHED] = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
+ TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED,
+ [IP_CT_RELATED] = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
+ TCA_FLOWER_KEY_CT_FLAGS_RELATED,
+ [IP_CT_ESTABLISHED_REPLY] = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
+ TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED,
+ [IP_CT_RELATED_REPLY] = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
+ TCA_FLOWER_KEY_CT_FLAGS_RELATED,
+ [IP_CT_NEW] = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
+ TCA_FLOWER_KEY_CT_FLAGS_NEW,
+};
+
static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct tcf_result *res)
{
struct cls_fl_head *head = rcu_dereference_bh(tp->root);
- struct cls_fl_filter *f;
- struct fl_flow_mask *mask;
- struct fl_flow_key skb_key;
struct fl_flow_key skb_mkey;
+ struct fl_flow_key skb_key;
+ struct fl_flow_mask *mask;
+ struct cls_fl_filter *f;
list_for_each_entry_rcu(mask, &head->masks, list) {
fl_clear_masked_range(&skb_key, mask);
@@ -290,6 +306,9 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
*/
skb_key.basic.n_proto = skb->protocol;
skb_flow_dissect_tunnel_info(skb, &mask->dissector, &skb_key);
+ skb_flow_dissect_ct(skb, &mask->dissector, &skb_key,
+ fl_ct_info_to_flower_map,
+ ARRAY_SIZE(fl_ct_info_to_flower_map));
skb_flow_dissect(skb, &mask->dissector, &skb_key, 0);
fl_set_masked_key(&skb_mkey, &skb_key, mask);
@@ -704,6 +723,16 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
[TCA_FLOWER_KEY_ENC_IP_TTL_MASK] = { .type = NLA_U8 },
[TCA_FLOWER_KEY_ENC_OPTS] = { .type = NLA_NESTED },
[TCA_FLOWER_KEY_ENC_OPTS_MASK] = { .type = NLA_NESTED },
+ [TCA_FLOWER_KEY_CT_STATE] = { .type = NLA_U16 },
+ [TCA_FLOWER_KEY_CT_STATE_MASK] = { .type = NLA_U16 },
+ [TCA_FLOWER_KEY_CT_ZONE] = { .type = NLA_U16 },
+ [TCA_FLOWER_KEY_CT_ZONE_MASK] = { .type = NLA_U16 },
+ [TCA_FLOWER_KEY_CT_MARK] = { .type = NLA_U32 },
+ [TCA_FLOWER_KEY_CT_MARK_MASK] = { .type = NLA_U32 },
+ [TCA_FLOWER_KEY_CT_LABELS] = { .type = NLA_BINARY,
+ .len = 128 / BITS_PER_BYTE },
+ [TCA_FLOWER_KEY_CT_LABELS_MASK] = { .type = NLA_BINARY,
+ .len = 128 / BITS_PER_BYTE },
};
static const struct nla_policy
@@ -725,11 +754,11 @@ static void fl_set_key_val(struct nlattr **tb,
{
if (!tb[val_type])
return;
- memcpy(val, nla_data(tb[val_type]), len);
+ nla_memcpy(val, tb[val_type], len);
if (mask_type == TCA_FLOWER_UNSPEC || !tb[mask_type])
memset(mask, 0xff, len);
else
- memcpy(mask, nla_data(tb[mask_type]), len);
+ nla_memcpy(mask, tb[mask_type], len);
}
static int fl_set_key_port_range(struct nlattr **tb, struct fl_flow_key *key,
@@ -1015,6 +1044,51 @@ static int fl_set_enc_opt(struct nlattr **tb, struct fl_flow_key *key,
return 0;
}
+static int fl_set_key_ct(struct nlattr **tb,
+ struct flow_dissector_key_ct *key,
+ struct flow_dissector_key_ct *mask,
+ struct netlink_ext_ack *extack)
+{
+ if (tb[TCA_FLOWER_KEY_CT_STATE]) {
+ if (!IS_ENABLED(CONFIG_NF_CONNTRACK)) {
+ NL_SET_ERR_MSG(extack, "Conntrack isn't enabled");
+ return -EOPNOTSUPP;
+ }
+ fl_set_key_val(tb, &key->ct_state, TCA_FLOWER_KEY_CT_STATE,
+ &mask->ct_state, TCA_FLOWER_KEY_CT_STATE_MASK,
+ sizeof(key->ct_state));
+ }
+ if (tb[TCA_FLOWER_KEY_CT_ZONE]) {
+ if (!IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)) {
+ NL_SET_ERR_MSG(extack, "Conntrack zones isn't enabled");
+ return -EOPNOTSUPP;
+ }
+ fl_set_key_val(tb, &key->ct_zone, TCA_FLOWER_KEY_CT_ZONE,
+ &mask->ct_zone, TCA_FLOWER_KEY_CT_ZONE_MASK,
+ sizeof(key->ct_zone));
+ }
+ if (tb[TCA_FLOWER_KEY_CT_MARK]) {
+ if (!IS_ENABLED(CONFIG_NF_CONNTRACK_MARK)) {
+ NL_SET_ERR_MSG(extack, "Conntrack mark isn't enabled");
+ return -EOPNOTSUPP;
+ }
+ fl_set_key_val(tb, &key->ct_mark, TCA_FLOWER_KEY_CT_MARK,
+ &mask->ct_mark, TCA_FLOWER_KEY_CT_MARK_MASK,
+ sizeof(key->ct_mark));
+ }
+ if (tb[TCA_FLOWER_KEY_CT_LABELS]) {
+ if (!IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS)) {
+ NL_SET_ERR_MSG(extack, "Conntrack labels aren't enabled");
+ return -EOPNOTSUPP;
+ }
+ fl_set_key_val(tb, key->ct_labels, TCA_FLOWER_KEY_CT_LABELS,
+ mask->ct_labels, TCA_FLOWER_KEY_CT_LABELS_MASK,
+ sizeof(key->ct_labels));
+ }
+
+ return 0;
+}
+
static int fl_set_key(struct net *net, struct nlattr **tb,
struct fl_flow_key *key, struct fl_flow_key *mask,
struct netlink_ext_ack *extack)
@@ -1224,6 +1298,10 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
return ret;
}
+ ret = fl_set_key_ct(tb, &key->ct, &mask->ct, extack);
+ if (ret)
+ return ret;
+
if (tb[TCA_FLOWER_KEY_FLAGS])
ret = fl_set_key_flags(tb, &key->control.flags, &mask->control.flags);
@@ -1322,6 +1400,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
FLOW_DISSECTOR_KEY_ENC_IP, enc_ip);
FL_KEY_SET_IF_MASKED(mask, keys, cnt,
FLOW_DISSECTOR_KEY_ENC_OPTS, enc_opts);
+ FL_KEY_SET_IF_MASKED(mask, keys, cnt,
+ FLOW_DISSECTOR_KEY_CT, ct);
skb_flow_dissector_init(dissector, keys, cnt);
}
@@ -2076,6 +2156,40 @@ static int fl_dump_key_geneve_opt(struct sk_buff *skb,
return -EMSGSIZE;
}
+static int fl_dump_key_ct(struct sk_buff *skb,
+ struct flow_dissector_key_ct *key,
+ struct flow_dissector_key_ct *mask)
+{
+ if (IS_ENABLED(CONFIG_NF_CONNTRACK) &&
+ fl_dump_key_val(skb, &key->ct_state, TCA_FLOWER_KEY_CT_STATE,
+ &mask->ct_state, TCA_FLOWER_KEY_CT_STATE_MASK,
+ sizeof(key->ct_state)))
+ goto nla_put_failure;
+
+ if (IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES) &&
+ fl_dump_key_val(skb, &key->ct_zone, TCA_FLOWER_KEY_CT_ZONE,
+ &mask->ct_zone, TCA_FLOWER_KEY_CT_ZONE_MASK,
+ sizeof(key->ct_zone)))
+ goto nla_put_failure;
+
+ if (IS_ENABLED(CONFIG_NF_CONNTRACK_MARK) &&
+ fl_dump_key_val(skb, &key->ct_mark, TCA_FLOWER_KEY_CT_MARK,
+ &mask->ct_mark, TCA_FLOWER_KEY_CT_MARK_MASK,
+ sizeof(key->ct_mark)))
+ goto nla_put_failure;
+
+ if (IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS) &&
+ fl_dump_key_val(skb, &key->ct_labels, TCA_FLOWER_KEY_CT_LABELS,
+ &mask->ct_labels, TCA_FLOWER_KEY_CT_LABELS_MASK,
+ sizeof(key->ct_labels)))
+ goto nla_put_failure;
+
+ return 0;
+
+nla_put_failure:
+ return -EMSGSIZE;
+}
+
static int fl_dump_key_options(struct sk_buff *skb, int enc_opt_type,
struct flow_dissector_key_enc_opts *enc_opts)
{
@@ -2309,6 +2423,9 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
fl_dump_key_enc_opt(skb, &key->enc_opts, &mask->enc_opts))
goto nla_put_failure;
+ if (fl_dump_key_ct(skb, &key->ct, &mask->ct))
+ goto nla_put_failure;
+
if (fl_dump_key_flags(skb, key->control.flags, mask->control.flags))
goto nla_put_failure;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 0/4] net/sched: Introduce tc connection tracking
From: Paul Blakey @ 2019-07-04 11:53 UTC (permalink / raw)
To: Jiri Pirko, Paul Blakey, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, netdev, David Miller, Aaron Conole,
Zhike Wang
Cc: Rony Efraim, nst-kernel, John Hurley, Simon Horman, Justin Pettit
Hi,
This patch series add connection tracking capabilities in tc sw datapath.
It does so via a new tc action, called act_ct, and new tc flower classifier matching
on conntrack state, mark and label.
Usage is as follows:
$ tc qdisc add dev ens1f0_0 ingress
$ tc qdisc add dev ens1f0_1 ingress
$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 0 proto ip \
flower ip_proto tcp ct_state -trk \
action ct zone 2 pipe \
action goto chain 2
$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 2 proto ip \
flower ct_state +trk+new \
action ct zone 2 commit mark 0xbb nat src addr 5.5.5.7 pipe \
action mirred egress redirect dev ens1f0_1
$ tc filter add dev ens1f0_0 ingress \
prio 1 chain 2 proto ip \
flower ct_zone 2 ct_mark 0xbb ct_state +trk+est \
action ct nat pipe \
action mirred egress redirect dev ens1f0_1
$ tc filter add dev ens1f0_1 ingress \
prio 1 chain 0 proto ip \
flower ip_proto tcp ct_state -trk \
action ct zone 2 pipe \
action goto chain 1
$ tc filter add dev ens1f0_1 ingress \
prio 1 chain 1 proto ip \
flower ct_zone 2 ct_mark 0xbb ct_state +trk+est \
action ct nat pipe \
action mirred egress redirect dev ens1f0_0
The pattern used in the design here closely resembles OvS, as the plan is to also offload
OvS conntrack rules to tc. OvS datapath rules uses it's recirculation mechanism to send
specific packets to conntrack, and return with the new conntrack state (ct_state) on some other recirc_id
to be matched again (we use goto chain for this).
This results in the following OvS datapath rules:
recirc_id(0),in_port(ens1f0_0),ct_state(-trk),... actions:ct(zone=2),recirc(2)
recirc_id(2),in_port(ens1f0_0),ct_state(+new+trk),ct_mark(0xbb),... actions:ct(commit,zone=2,nat(src=5.5.5.7),mark=0xbb),ens1f0_1
recirc_id(2),in_port(ens1f0_0),ct_state(+est+trk),ct_mark(0xbb),... actions:ct(zone=2,nat),ens1f0_1
recirc_id(1),in_port(ens1f0_1),ct_state(-trk),... actions:ct(zone=2),recirc(1)
recirc_id(1),in_port(ens1f0_1),ct_state(+est+trk),... actions:ct(zone=2,nat),ens1f0_0
Changelog:
See individual patches.
Paul Blakey (4):
net/sched: Introduce action ct
net/flow_dissector: add connection tracking dissection
net/sched: cls_flower: Add matching on conntrack info
tc-tests: Add tc action ct tests
include/linux/skbuff.h | 10 +
include/net/flow_dissector.h | 15 +
include/net/flow_offload.h | 5 +
include/net/tc_act/tc_ct.h | 64 ++
include/uapi/linux/pkt_cls.h | 17 +
include/uapi/linux/tc_act/tc_ct.h | 41 +
net/core/flow_dissector.c | 44 +
net/sched/Kconfig | 11 +
net/sched/Makefile | 1 +
net/sched/act_ct.c | 978 +++++++++++++++++++++
net/sched/cls_api.c | 5 +
net/sched/cls_flower.c | 127 ++-
.../selftests/tc-testing/tc-tests/actions/ct.json | 314 +++++++
13 files changed, 1627 insertions(+), 5 deletions(-)
create mode 100644 include/net/tc_act/tc_ct.h
create mode 100644 include/uapi/linux/tc_act/tc_ct.h
create mode 100644 net/sched/act_ct.c
create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/ct.json
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Michal Kubecek @ 2019-07-04 11:52 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190704080435.GF2250@nanopsycho>
On Thu, Jul 04, 2019 at 10:04:35AM +0200, Jiri Pirko wrote:
> Wed, Jul 03, 2019 at 08:18:51PM CEST, mkubecek@suse.cz wrote:
> >On Wed, Jul 03, 2019 at 01:49:33PM +0200, Jiri Pirko wrote:
> >> Tue, Jul 02, 2019 at 01:50:09PM CEST, mkubecek@suse.cz wrote:
> >> >+Compact form: nested (bitset) atrribute contents:
> >> >+
> >> >+ ETHTOOL_A_BITSET_LIST (flag) no mask, only a list
> >> >+ ETHTOOL_A_BITSET_SIZE (u32) number of significant bits
> >> >+ ETHTOOL_A_BITSET_VALUE (binary) bitmap of bit values
> >> >+ ETHTOOL_A_BITSET_MASK (binary) bitmap of valid bits
> >> >+
> >> >+Value and mask must have length at least ETHTOOL_A_BITSET_SIZE bits rounded up
> >> >+to a multiple of 32 bits. They consist of 32-bit words in host byte order,
> >>
> >> Looks like the blocks are similar to NLA_BITFIELD32. Why don't you user
> >> nested array of NLA_BITFIELD32 instead?
> >
> >That would mean a layout like
> >
> > 4 bytes of attr header
> > 4 bytes of value
> > 4 bytes of mask
> > 4 bytes of attr header
> > 4 bytes of value
> > 4 bytes of mask
> > ...
> >
> >i.e. interleaved headers, words of value and words of mask. Having value
> >and mask contiguous looks cleaner to me. Also, I can quickly check the
> >sizes without iterating through a (potentially long) array.
>
> Yeah, if you are not happy with this, I suggest to introduce
> NLA_BITFIELD with arbitrary size. That would be probably cleanest.
There is still the question if it it should be implemented as a nested
attribute which could look like the current compact form without the
"list" flag (if there is no mask, it's a list). Or an unstructured data
block consisting of u32 bit length and one or two bitmaps of
corresponding length. I would prefer the nested attribute, netlink was
designed to represent structured data, passing structures as binary goes
against the design (just looked at VFINFO in rtnetlink few days ago,
it's awful, IMHO).
Either way, I would still prefer to have bitmaps represented as an array
of 32-bit blocks in host byte order. This would be easy to handle in
kernel both in places where we have u32 based bitmaps and unsigned long
based ones. Other options seem less appealing:
- u8 based: only complicates processing
- u64 based: have to care about alignment
- unsigned long based: alignment and also problems with 64-bit kernel
vs. 32-bit userspace
> >> This is quite complex and confusing. Having the same API for 2 APIs is
> >> odd. The API should be crystal clear, easy to use.
> >>
> >> Why can't you have 2 commands, one working with bit arrays only, one
> >> working with strings? Something like:
> >> X_GET
> >> ETHTOOL_A_BITS (nested)
> >> ETHTOOL_A_BIT_ARRAY (BITFIELD32)
> >> X_NAMES_GET
> >> ETHTOOL_A_BIT_NAMES (nested)
> >> ETHTOOL_A_BIT_INDEX
> >> ETHTOOL_A_BIT_NAME
> >>
> >> For set, you can also have multiple cmds:
> >> X_SET - to set many at once, by bit index
> >> ETHTOOL_A_BITS (nested)
> >> ETHTOOL_A_BIT_ARRAY (BITFIELD32)
> >> X_ONE_SET - to set one, by bit index
> >> ETHTOOL_A_BIT_INDEX
> >> ETHTOOL_A_BIT_VALUE
> >> X_ONE_SET - to set one, by name
> >> ETHTOOL_A_BIT_NAME
> >> ETHTOOL_A_BIT_VALUE
> >
> >This looks as if you assume there is nothing except the bitset in the
> >message but that is not true. Even with your proposed breaking of
> >current groups, you would still have e.g. 4 bitsets in reply to netdev
> >features query, 3 in timestamping info GET request and often bitsets
> >combined with other data (e.g. WoL modes and optional WoL password).
> >If you wanted to further refine the message granularity to the level of
> >single parameters, we might be out of message type ids already.
>
> You can still have multiple bitsets(bitfields) in single message and
> have separate cmd/cmds to get string-bit mapping. No need to mangle it.
Let's take a look at what it means in practice, the command is
ethtool --set-prif-flags eth3 legacy-rx on
on an ixgbe card. Currently, ethtool (from the github repository) does
------------------------------------------------------------------------
ETHTOOL_CMD_SETTINGS_SET (K->U, 68 bytes)
ETHTOOL_A_HEADER
ETHTOOL_A_DEV_NAME = "eth3"
ETHTOOL_A_SETTINGS_PRIV_FLAGS
ETHTOOL_A_BITSET_BITS
ETHTOOL_A_BITS_BIT
ETHTOOL_A_BIT_NAME = "legacy-rx"
ETHTOOL_A_BIT_VALUE
NLMSG_ERR (K->U, 36 bytes) err = 0
------------------------------------------------------------------------
If we had only compact form (or some of the NLA_BITFIELD solutions we
are talking about), you would need
------------------------------------------------------------------------
ETHTOOL_CMD_STRSET_GET (U->K, 52 bytes)
ETHTOOL_A_HEADER
ETHTOOL_A_DEV_NAME = "eth3"
ETHTOOL_A_STRSET_STRINGSETS
ETHTOOL_A_STRINGSETS_STRINGSET
ETHTOOL_A_STRINGSET_ID = 2 (ETH_SS_PRIV_FLAGS)
ETHTOOL_CMD_STRSET_GET_REPLY (K->U, 128 bytes)
ETHTOOL_A_HEADER
ETHTOOL_A_DEV_INDEX = 9
ETHTOOL_A_DEV_NAME = "eth3"
ETHTOOL_A_STRSET_STRINGSETS
ETHTOOL_A_STRINGSETS_STRINGSET
ETHTOOL_A_STRINGSET_ID = 2 (ETH_SS_PRIV_FLAGS)
ETHTOOL_A_STRINGSET_COUNT = 2
ETHTOOL_A_STRINGSET_STRINGS
ETHTOOL_A_STRINGS_STRING
ETHTOOL_A_STRING_INDEX = 0
ETHTOOL_A_STRING_VALUE = "legacy-rx"
ETHTOOL_A_STRINGS_STRING
ETHTOOL_A_STRING_INDEX = 1
ETHTOOL_A_STRING_VALUE = "vf-ipsec"
NLMSG_ERR (K->U, 36 bytes) err = 0
ETHTOOL_CMD_SETTINGS_SET (K->U, 64 bytes)
ETHTOOL_A_HEADER
ETHTOOL_A_DEV_NAME = "eth3"
ETHTOOL_A_SETTINGS_PRIV_FLAGS
ETHTOOL_A_BITSET_SIZE = 2
ETHTOOL_A_BITSET_VALUE = 00000001
ETHTOOL_A_BITSET_MASK = 00000001
NLMSG_ERR (K->U, 36 bytes) err = 0
------------------------------------------------------------------------
That's an extra roundtrip, lot more chat and the SETTINGS_SET message is
only 4 bytes shorter in the end. And we can consider ourselves lucky
this NIC has only two private flags. Or that we didn't need to enable or
disable a netdev feature (56 bits) or link mode (69 bits and growing).
We could reduce the overhead by allowing STRSET_GET query to only ask
for specific string(s) but there would still be the extra roundtrip
which I dislike in the ioctl interface. Florian also said in the v5
discussion that he would like if it was possible to get names and data
together in one request.
Michal
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Ilias Apalodimas @ 2019-07-04 11:11 UTC (permalink / raw)
To: Jose Abreu
Cc: Jesper Dangaard Brouer, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai
In-Reply-To: <BN8PR12MB3266BC5322AADFAC49D9BAFAD3FA0@BN8PR12MB3266.namprd12.prod.outlook.com>
On Thu, Jul 04, 2019 at 10:13:37AM +0000, Jose Abreu wrote:
> From: Jesper Dangaard Brouer <brouer@redhat.com>
>
> > The page_pool DMA mapping cannot be "kept" when page traveling into the
> > network stack attached to an SKB. (Ilias and I have a long term plan[1]
> > to allow this, but you cannot do it ATM).
>
> The reason I recycle the page is this previous call to:
>
> skb_copy_to_linear_data()
>
> So, technically, I'm syncing to CPU the page(s) and then memcpy to a
> previously allocated SKB ... So it's safe to just recycle the mapping I
> think.
>
> Its kind of using bounce buffers and I do see performance gain in this
> (I think the reason is because my setup uses swiotlb for DMA mapping).
Maybe. Have you tested this on big/small packets?
Can you do a test with 64b/128b and 1024b for example?
The memcpy might be cheap for the small sized packets (and cheaper than the dma
map/unmap)
>
> Anyway, I'm open to some suggestions on how to improve this ...
>
> > Also remember that the page_pool requires you driver to do the DMA-sync
> > operation. I see a dma_sync_single_for_cpu(), but I didn't see a
> > dma_sync_single_for_device() (well, I noticed one getting removed).
> > (For some HW Ilias tells me that the dma_sync_single_for_device can be
> > elided, so maybe this can still be correct for you).
>
> My HW just needs descriptors refilled which are in different coherent
> region so I don't see any reason for dma_sync_single_for_device() ...
I am abit overloaded at the moment. I'll try to have a look at this and get back
to you
Cheers
/Ilias
^ permalink raw reply
* [PATCH v2] rtl8xxxu: Fix wifi low signal strength issue of RTL8723BU
From: Chris Chiu @ 2019-07-04 10:55 UTC (permalink / raw)
To: jes.sorensen, kvalo, davem; +Cc: linux-wireless, netdev, linux-kernel, linux
The WiFi tx power of RTL8723BU is extremely low after booting. So
the WiFi scan gives very limited AP list and it always fails to
connect to the selected AP. This module only supports 1x1 antenna
and the antenna is switched to bluetooth due to some incorrect
register settings.
Compare with the vendor driver https://github.com/lwfinger/rtl8723bu,
we realized that the 8723bu's enable_rf() does the same thing as
rtw_btcoex_HAL_Initialize() in vendor driver. And it by default
sets the antenna path to BTC_ANT_PATH_BT which we verified it's
the cause of the wifi weak tx power. The vendor driver will set
the antenna path to BTC_ANT_PATH_PTA in the consequent btcoexist
mechanism, by the function halbtc8723b1ant_PsTdma.
This commit hand over the antenna control to PTA(Packet Traffic
Arbitration), which compares the weight of bluetooth/wifi traffic
then determine whether to continue current wifi traffic or not.
After PTA take control, The wifi signal will be back to normal and
the bluetooth scan can also work at the same time. However, the
btcoexist still needs to be handled under different circumstances.
If there's a BT connection established, the wifi still fails to
connect until BT disconnected.
Signed-off-by: Chris Chiu <chiu@endlessm.com>
---
Note:
v2:
- Replace BIT(11) with the descriptive definition
- Meaningful comment for the REG_S0S1_PATH_SWITCH setting
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c | 11 ++++++++---
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 3 ++-
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
index 3adb1d3d47ac..ceffe05bd65b 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8723b.c
@@ -1525,7 +1525,7 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
/*
* WLAN action by PTA
*/
- rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x04);
+ rtl8xxxu_write8(priv, REG_WLAN_ACT_CONTROL_8723B, 0x0c);
/*
* BT select S0/S1 controlled by WiFi
@@ -1568,9 +1568,14 @@ static void rtl8723b_enable_rf(struct rtl8xxxu_priv *priv)
rtl8xxxu_gen2_h2c_cmd(priv, &h2c, sizeof(h2c.ant_sel_rsv));
/*
- * 0x280, 0x00, 0x200, 0x80 - not clear
+ * Different settings per different antenna position.
+ * Antenna Position: | Normal Inverse
+ * --------------------------------------------------
+ * Antenna switch to BT: | 0x280, 0x00
+ * Antenna switch to WiFi: | 0x0, 0x280
+ * Antenna switch to PTA: | 0x200, 0x80
*/
- rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x00);
+ rtl8xxxu_write32(priv, REG_S0S1_PATH_SWITCH, 0x80);
/*
* Software control, antenna at WiFi side
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 8136e268b4e6..c6c41fb962ff 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -3891,12 +3891,13 @@ static int rtl8xxxu_init_device(struct ieee80211_hw *hw)
/* Check if MAC is already powered on */
val8 = rtl8xxxu_read8(priv, REG_CR);
+ val16 = rtl8xxxu_read16(priv, REG_SYS_CLKR);
/*
* Fix 92DU-VC S3 hang with the reason is that secondary mac is not
* initialized. First MAC returns 0xea, second MAC returns 0x00
*/
- if (val8 == 0xea)
+ if (val8 == 0xea || !(val16 & SYS_CLK_MAC_CLK_ENABLE))
macpower = false;
else
macpower = true;
--
2.11.0
^ permalink raw reply related
* Re: bug: tpacket_snd can cause data corruption
From: Frank de Brabander @ 2019-07-04 10:43 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: David S . Miller, Willem de Bruijn, Network Development
In-Reply-To: <CAF=yD-+wHzfP6QWJzc=num_VaFvN3RYXV-c3+-VY8EjS87WEiA@mail.gmail.com>
On 03-07-19 18:07, Willem de Bruijn wrote:
> On Wed, Jul 3, 2019 at 7:08 AM Frank de Brabander <debrabander@gmail.com> wrote:
>> In commit 5cd8d46e a fix was applied for data corruption in
>> tpacket_snd. A selftest was added in commit 358be656 which
>> validates this fix.
>>
>> Unfortunately this bug still persists, although since this fix less
>> likely to trigger. This bug was initially observed using a PACKET_MMAP
>> application, but can also be seen by tweaking the kernel selftest.
>>
>> By tweaking the selftest txring_overwrite.c to run
>> as an infinite loop, the data corruption will still trigger. It
>> seems to occur faster by generating interrupts (e.g. by plugging
>> in USB devices). Tested with kernel version 5.2-RC7.
>>
>> Cause for this bug is still unclear.
> The cause of the original bug is well understood.
>
> The issue you report I expect is due to background traffic. And more
> about the test than the kernel implementation.
>
> Can you reproduce the issue when running the modified test in a
> network namespace (./in_netns.sh ./txring_overwrite)?
>
> I observe the issue report outside that, but not inside. That implies
> that what we're observing is random background traffic. The modified
> test then drops the unexpected packet because it mismatches on length.
> As a result the next read (the test always sends two packets, then
> reads both) will report a data mismatch. Because it is reading the
> first test packet, but expecting the second. Output with a bit more
> data:
>
> count: 200
> count: 300
> count: 400
> count: 500
> read: 90B != 100B
> wrong pattern: 0x61 != 0x62
> count: 600
> count: 700
> count: 800
> read: 90B != 100B
> wrong pattern: 0x61 != 0x62
> count: 900
> read: 90B != 100B
> wrong pattern: 0x61 != 0x62
>
> Notice the clear pattern.
>
> This does not trigger inside a network namespace, which is how
> kselftest invokes txring_override (from run_afpackettests).
I'm also able to reproduce the issue inside a network namespace.
I've added the extra logging, as seen in your output, for
mismatches on length. Running the test without ./in_netns.sh
is indeed as you describe:
read error: 66 != 100
wrong pattern: 0x61 != 0x62
read error: 66 != 100
wrong pattern: 0x61 != 0x62
read error: 74 != 100
read error: 66 != 100
wrong pattern: 0x53 != 0x61
wrong pattern: 0x53 != 0x62
read error: 66 != 100
read error: 66 != 100
read error: 66 != 100
wrong pattern: 0x61 != 0x62
read error: 95 != 100
read error: 95 != 100
wrong pattern: 0xffffffbe != 0x61
wrong pattern: 0x61 != 0x62
read error: 66 != 100
But even when running the test with ./in_netns.sh it shows
"wrong pattern", this time without length mismatches:
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
wrong pattern: 0x62 != 0x61
As already mentioned, it seems to trigger mainly (only ?) when
an USB device is connected. The PC I'm testing this on has an
USB hub with many ports and connected devices. When connecting
this USB hub, the amount of "wrong pattern" errors that are
shown seems to correlate to the amount of new devices
that the kernel should detect. Connecting in a single USB device
also triggers the error, but not on every attempt.
Unfortunately have not found any other way to force the
error to trigger. E.g. running stress-ng to generate CPU load or
timer interrupts does not seem to have any impact.
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Ilias Apalodimas @ 2019-07-04 10:30 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Jose Abreu, linux-kernel, netdev, linux-stm32, linux-arm-kernel,
Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai
In-Reply-To: <20190704120018.4523a119@carbon>
HI Jesper, Ivan,
> On Wed, 3 Jul 2019 12:37:50 +0200
> Jose Abreu <Jose.Abreu@synopsys.com> wrote:
>
> > @@ -3547,6 +3456,9 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
> >
> > napi_gro_receive(&ch->rx_napi, skb);
> >
> > + page_pool_recycle_direct(rx_q->page_pool, buf->page);
>
> This doesn't look correct.
>
> The page_pool DMA mapping cannot be "kept" when page traveling into the
> network stack attached to an SKB. (Ilias and I have a long term plan[1]
> to allow this, but you cannot do it ATM).
>
> You will have to call:
> page_pool_release_page(rx_q->page_pool, buf->page);
>
> This will do a DMA-unmap, and you will likely loose your performance
> gain :-(
>
>
> > + buf->page = NULL;
> > +
> > priv->dev->stats.rx_packets++;
> > priv->dev->stats.rx_bytes += frame_len;
> > }
>
> Also remember that the page_pool requires you driver to do the DMA-sync
> operation. I see a dma_sync_single_for_cpu(), but I didn't see a
> dma_sync_single_for_device() (well, I noticed one getting removed).
> (For some HW Ilias tells me that the dma_sync_single_for_device can be
> elided, so maybe this can still be correct for you).
On our case (and in the page_pool API in general) you have to track buffers when
both .ndo_xdp_xmit() and XDP_TX are used.
So the lifetime of a packet might be
1. page pool allocs packet. The API doesn't sync but i *think* you don't have to
explicitly since the CPU won't touch that buffer until the NAPI handler kicks
in. On the napi handler you need to dma_sync_single_for_cpu() and process the
packet.
2a) no XDP is required so the packet is unmapped and free'd
2b) .ndo_xdp_xmit is called so tyhe buffer need to be mapped/unmapped
2c) XDP_TX is called. In that case we re-use an Rx buffer so we need to
dma_sync_single_for_device()
2a and 2b won't cause any issues
In 2c the buffer will be recycled and fed back to the device with a *correct*
sync (for_device) and all those buffers are allocated as DMA_BIDIRECTIONAL.
So bvottom line i *think* we can skip the dma_sync_single_for_device() on the
initial allocation *only*. If am terribly wrong please let me know :)
Thanks
/Ilias
>
>
> [1] https://github.com/xdp-project/xdp-project/blob/master/areas/mem/page_pool02_SKB_return_callback.org
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v6 net-next 1/5] xdp: allow same allocator usage
From: Ivan Khoronzhuk @ 2019-07-04 10:22 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: grygorii.strashko, hawk, davem, ast, linux-kernel, linux-omap,
xdp-newbies, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend
In-Reply-To: <20190703194013.02842e42@carbon>
On Wed, Jul 03, 2019 at 07:40:13PM +0200, Jesper Dangaard Brouer wrote:
>On Wed, 3 Jul 2019 13:18:59 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> First of all, it is an absolute requirement that each RX-queue have
>> their own page_pool object/allocator. And this change is intendant
>> to handle special case, where a single RX-queue can receive packets
>> from two different net_devices.
>>
>> In order to protect against using same allocator for 2 different rx
>> queues, add queue_index to xdp_mem_allocator to catch the obvious
>> mistake where queue_index mismatch, as proposed by Jesper Dangaard
>> Brouer.
>>
>> Adding this on xdp allocator level allows drivers with such dependency
>> change the allocators w/o modifications.
>>
>> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> ---
>> include/net/xdp_priv.h | 2 ++
>> net/core/xdp.c | 55 ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 57 insertions(+)
>>
>> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
>> index 6a8cba6ea79a..9858a4057842 100644
>> --- a/include/net/xdp_priv.h
>> +++ b/include/net/xdp_priv.h
>> @@ -18,6 +18,8 @@ struct xdp_mem_allocator {
>> struct rcu_head rcu;
>> struct delayed_work defer_wq;
>> unsigned long defer_warn;
>> + unsigned long refcnt;
>> + u32 queue_index;
>> };
>
>I don't like this approach, because I think we need to extend struct
>xdp_mem_allocator with a net_device pointer, for doing dev_hold(), to
>correctly handle lifetime issues. (As I tried to explain previously).
>This will be much harder after this change, which is why I proposed the
>other patch.
My concern comes not from zero also.
It's partly continuation of not answered questions from here:
https://lwn.net/ml/netdev/20190625122822.GC6485@khorivan/
"For me it's important to know only if it means that alloc.count is
freed at first call of __mem_id_disconnect() while shutdown.
The workqueue for the rest is connected only with ring cache protected
by ring lock and not supposed that alloc.count can be changed while
workqueue tries to shutdonwn the pool."
So patch you propose to leave works only because of luck, because fast
cache is cleared before workqueue is scheduled and no races between two
workqueues for fast cache later. I'm not really against this patch, but
I have to try smth better.
So, the patch is fine only because of specific of page_pool implementation.
I'm not sure that in future similar workqueue completion will be lucky for
another allocator (it easily can happen due to xdp frame can live longer
than an allocator). Similar problem can happen with other drivers having
same allocator, that can use zca (potentially can use smth similar),
af_xdp api allows to switch on it or some other allocators....
But not the essence. The concern about adding smth new to the allocator
later, like net device, can be solved with a little modification to the patch,
(despite here can be several more approaches) for instance, like this:
(by fact it's still the same, when mem_alloc instance per each register call
but with same void *allocator)
diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
index 6a8cba6ea79a..c7ad0f41e1b0 100644
--- a/include/net/xdp_priv.h
+++ b/include/net/xdp_priv.h
@@ -18,6 +18,8 @@ struct xdp_mem_allocator {
struct rcu_head rcu;
struct delayed_work defer_wq;
unsigned long defer_warn;
+ unsigned long *refcnt;
+ u32 queue_index;
};
#endif /* __LINUX_NET_XDP_PRIV_H__ */
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 829377cc83db..a44e3e4c8307 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -64,9 +64,37 @@ static const struct rhashtable_params mem_id_rht_params = {
.obj_cmpfn = xdp_mem_id_cmp,
};
+static struct xdp_mem_allocator *xdp_allocator_find(void *allocator)
+{
+ struct xdp_mem_allocator *xae, *xa = NULL;
+ struct rhashtable_iter iter;
+
+ if (!allocator)
+ return xa;
+
+ rhashtable_walk_enter(mem_id_ht, &iter);
+ do {
+ rhashtable_walk_start(&iter);
+
+ while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
+ if (xae->allocator == allocator) {
+ xa = xae;
+ break;
+ }
+ }
+
+ rhashtable_walk_stop(&iter);
+
+ } while (xae == ERR_PTR(-EAGAIN));
+ rhashtable_walk_exit(&iter);
+
+ return xa;
+}
+
static void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
{
struct xdp_mem_allocator *xa;
+ void *allocator;
xa = container_of(rcu, struct xdp_mem_allocator, rcu);
@@ -74,15 +102,27 @@ static void __xdp_mem_allocator_rcu_free(struct rcu_head *rcu)
if (xa->mem.type == MEM_TYPE_PAGE_POOL)
page_pool_free(xa->page_pool);
- /* Allow this ID to be reused */
- ida_simple_remove(&mem_id_pool, xa->mem.id);
+ kfree(xa->refcnt);
+ allocator = xa->allocator;
+ while (xa) {
+ xa = xdp_allocator_find(allocator);
+ if (!xa)
+ break;
+
+ mutex_lock(&mem_id_lock);
+ rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params);
+ mutex_unlock(&mem_id_lock);
- /* Poison memory */
- xa->mem.id = 0xFFFF;
- xa->mem.type = 0xF0F0;
- xa->allocator = (void *)0xDEAD9001;
+ /* Allow this ID to be reused */
+ ida_simple_remove(&mem_id_pool, xa->mem.id);
- kfree(xa);
+ /* Poison memory */
+ xa->mem.id = 0xFFFF;
+ xa->mem.type = 0xF0F0;
+ xa->allocator = (void *)0xDEAD9001;
+
+ kfree(xa);
+ }
}
static bool __mem_id_disconnect(int id, bool force)
@@ -98,6 +138,18 @@ static bool __mem_id_disconnect(int id, bool force)
WARN(1, "Request remove non-existing id(%d), driver bug?", id);
return true;
}
+
+ /* to avoid calling hash lookup twice, decrement refcnt here till it
+ * reaches zero, then it can be called from workqueue afterwards.
+ */
+ if (*xa->refcnt)
+ (*xa->refcnt)--;
+
+ if (*xa->refcnt) {
+ mutex_unlock(&mem_id_lock);
+ return true;
+ }
+
xa->disconnect_cnt++;
/* Detects in-flight packet-pages for page_pool */
@@ -106,8 +158,7 @@ static bool __mem_id_disconnect(int id, bool force)
trace_mem_disconnect(xa, safe_to_remove, force);
- if ((safe_to_remove || force) &&
- !rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params))
+ if (safe_to_remove || force)
call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
mutex_unlock(&mem_id_lock);
@@ -316,6 +367,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
enum xdp_mem_type type, void *allocator)
{
struct xdp_mem_allocator *xdp_alloc;
+ unsigned long *refcnt = NULL;
gfp_t gfp = GFP_KERNEL;
int id, errno, ret;
void *ptr;
@@ -347,6 +399,19 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
}
}
+ mutex_lock(&mem_id_lock);
+ xdp_alloc = xdp_allocator_find(allocator);
+ if (xdp_alloc) {
+ /* One allocator per queue is supposed only */
+ if (xdp_alloc->queue_index != xdp_rxq->queue_index) {
+ mutex_unlock(&mem_id_lock);
+ return -EINVAL;
+ }
+
+ refcnt = xdp_alloc->refcnt;
+ }
+ mutex_unlock(&mem_id_lock);
+
xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
if (!xdp_alloc)
return -ENOMEM;
@@ -360,6 +425,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
xdp_rxq->mem.id = id;
xdp_alloc->mem = xdp_rxq->mem;
xdp_alloc->allocator = allocator;
+ xdp_alloc->queue_index = xdp_rxq->queue_index;
/* Insert allocator into ID lookup table */
ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
@@ -370,6 +436,16 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
goto err;
}
+ if (!refcnt) {
+ refcnt = kzalloc(sizeof(*xdp_alloc->refcnt), gfp);
+ if (!refcnt) {
+ errno = -ENOMEM;
+ goto err;
+ }
+ }
+
+ (*refcnt)++;
+ xdp_alloc->refcnt = refcnt;
mutex_unlock(&mem_id_lock);
trace_mem_connect(xdp_alloc, xdp_rxq);
>
>
>> #endif /* __LINUX_NET_XDP_PRIV_H__ */
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index 829377cc83db..4f0ddbb3717a 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -98,6 +98,18 @@ static bool __mem_id_disconnect(int id, bool force)
>> WARN(1, "Request remove non-existing id(%d), driver bug?", id);
>> return true;
>> }
>> +
>> + /* to avoid calling hash lookup twice, decrement refcnt here till it
>> + * reaches zero, then it can be called from workqueue afterwards.
>> + */
>> + if (xa->refcnt)
>> + xa->refcnt--;
>> +
>> + if (xa->refcnt) {
>> + mutex_unlock(&mem_id_lock);
>> + return true;
>> + }
>> +
>> xa->disconnect_cnt++;
>>
>> /* Detects in-flight packet-pages for page_pool */
>> @@ -312,6 +324,33 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
>> return true;
>> }
>>
>> +static struct xdp_mem_allocator *xdp_allocator_find(void *allocator)
>> +{
>> + struct xdp_mem_allocator *xae, *xa = NULL;
>> + struct rhashtable_iter iter;
>> +
>> + if (!allocator)
>> + return xa;
>> +
>> + rhashtable_walk_enter(mem_id_ht, &iter);
>> + do {
>> + rhashtable_walk_start(&iter);
>> +
>> + while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
>> + if (xae->allocator == allocator) {
>> + xa = xae;
>> + break;
>> + }
>> + }
>> +
>> + rhashtable_walk_stop(&iter);
>> +
>> + } while (xae == ERR_PTR(-EAGAIN));
>> + rhashtable_walk_exit(&iter);
>> +
>> + return xa;
>> +}
>> +
>> int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>> enum xdp_mem_type type, void *allocator)
>> {
>> @@ -347,6 +386,20 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>> }
>> }
>>
>> + mutex_lock(&mem_id_lock);
>> + xdp_alloc = xdp_allocator_find(allocator);
>> + if (xdp_alloc) {
>> + /* One allocator per queue is supposed only */
>> + if (xdp_alloc->queue_index != xdp_rxq->queue_index)
>> + return -EINVAL;
>> +
>> + xdp_rxq->mem.id = xdp_alloc->mem.id;
>> + xdp_alloc->refcnt++;
>> + mutex_unlock(&mem_id_lock);
>> + return 0;
>> + }
>> + mutex_unlock(&mem_id_lock);
>> +
>> xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
>> if (!xdp_alloc)
>> return -ENOMEM;
>> @@ -360,6 +413,8 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>> xdp_rxq->mem.id = id;
>> xdp_alloc->mem = xdp_rxq->mem;
>> xdp_alloc->allocator = allocator;
>> + xdp_alloc->refcnt = 1;
>> + xdp_alloc->queue_index = xdp_rxq->queue_index;
>>
>> /* Insert allocator into ID lookup table */
>> ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
>
>
>
>--
>Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply related
* Re: [PATCH ipsec v2] xfrm interface: fix memory leak on creation
From: Steffen Klassert @ 2019-07-04 10:22 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: davem, netdev, Lorenzo Colitti, Benedict Wong, Shannon Nelson,
Antony Antony, Eyal Birger, Julien Floret
In-Reply-To: <20190702155139.11399-1-nicolas.dichtel@6wind.com>
On Tue, Jul 02, 2019 at 05:51:39PM +0200, Nicolas Dichtel wrote:
> The following commands produce a backtrace and return an error but the xfrm
> interface is created (in the wrong netns):
> $ ip netns add foo
> $ ip netns add bar
> $ ip -n foo netns set bar 0
> $ ip -n foo link add xfrmi0 link-netnsid 0 type xfrm dev lo if_id 23
> RTNETLINK answers: Invalid argument
> $ ip -n bar link ls xfrmi0
> 2: xfrmi0@lo: <NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> link/none 00:00:00:00:00:00 brd 00:00:00:00:00:00
>
> Here is the backtrace:
> [ 79.879174] WARNING: CPU: 0 PID: 1178 at net/core/dev.c:8172 rollback_registered_many+0x86/0x3c1
> [ 79.880260] Modules linked in: xfrm_interface nfsv3 nfs_acl auth_rpcgss nfsv4 nfs lockd grace sunrpc fscache button parport_pc parport serio_raw evdev pcspkr loop ext4 crc16 mbcache jbd2 crc32c_generic ide_cd_mod ide_gd_mod cdrom ata_$
> eneric ata_piix libata scsi_mod 8139too piix psmouse i2c_piix4 ide_core 8139cp mii i2c_core floppy
> [ 79.883698] CPU: 0 PID: 1178 Comm: ip Not tainted 5.2.0-rc6+ #106
> [ 79.884462] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> [ 79.885447] RIP: 0010:rollback_registered_many+0x86/0x3c1
> [ 79.886120] Code: 01 e8 d7 7d c6 ff 0f 0b 48 8b 45 00 4c 8b 20 48 8d 58 90 49 83 ec 70 48 8d 7b 70 48 39 ef 74 44 8a 83 d0 04 00 00 84 c0 75 1f <0f> 0b e8 61 cd ff ff 48 b8 00 01 00 00 00 00 ad de 48 89 43 70 66
> [ 79.888667] RSP: 0018:ffffc900015ab740 EFLAGS: 00010246
> [ 79.889339] RAX: ffff8882353e5700 RBX: ffff8882353e56a0 RCX: ffff8882353e5710
> [ 79.890174] RDX: ffffc900015ab7e0 RSI: ffffc900015ab7e0 RDI: ffff8882353e5710
> [ 79.891029] RBP: ffffc900015ab7e0 R08: ffffc900015ab7e0 R09: ffffc900015ab7e0
> [ 79.891866] R10: ffffc900015ab7a0 R11: ffffffff82233fec R12: ffffc900015ab770
> [ 79.892728] R13: ffffffff81eb7ec0 R14: ffff88822ed6cf00 R15: 00000000ffffffea
> [ 79.893557] FS: 00007ff350f31740(0000) GS:ffff888237a00000(0000) knlGS:0000000000000000
> [ 79.894581] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 79.895317] CR2: 00000000006c8580 CR3: 000000022c272000 CR4: 00000000000006f0
> [ 79.896137] Call Trace:
> [ 79.896464] unregister_netdevice_many+0x12/0x6c
> [ 79.896998] __rtnl_newlink+0x6e2/0x73b
> [ 79.897446] ? __kmalloc_node_track_caller+0x15e/0x185
> [ 79.898039] ? pskb_expand_head+0x5f/0x1fe
> [ 79.898556] ? stack_access_ok+0xd/0x2c
> [ 79.899009] ? deref_stack_reg+0x12/0x20
> [ 79.899462] ? stack_access_ok+0xd/0x2c
> [ 79.899927] ? stack_access_ok+0xd/0x2c
> [ 79.900404] ? __module_text_address+0x9/0x4f
> [ 79.900910] ? is_bpf_text_address+0x5/0xc
> [ 79.901390] ? kernel_text_address+0x67/0x7b
> [ 79.901884] ? __kernel_text_address+0x1a/0x25
> [ 79.902397] ? unwind_get_return_address+0x12/0x23
> [ 79.903122] ? __cmpxchg_double_slab.isra.37+0x46/0x77
> [ 79.903772] rtnl_newlink+0x43/0x56
> [ 79.904217] rtnetlink_rcv_msg+0x200/0x24c
>
> In fact, each time a xfrm interface was created, a netdev was allocated
> by __rtnl_newlink()/rtnl_create_link() and then another one by
> xfrmi_newlink()/xfrmi_create(). Only the second one was registered, it's
> why the previous commands produce a backtrace: dev_change_net_namespace()
> was called on a netdev with reg_state set to NETREG_UNINITIALIZED (the
> first one).
>
> CC: Lorenzo Colitti <lorenzo@google.com>
> CC: Benedict Wong <benedictwong@google.com>
> CC: Steffen Klassert <steffen.klassert@secunet.com>
> CC: Shannon Nelson <shannon.nelson@oracle.com>
> CC: Antony Antony <antony@phenome.org>
> CC: Eyal Birger <eyal.birger@gmail.com>
> Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
> Reported-by: Julien Floret <julien.floret@6wind.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Applied, thanks a lot!
^ permalink raw reply
* Re: [PATCH ipsec] xfrm: policy: fix bydst hlist corruption on hash rebuild
From: Steffen Klassert @ 2019-07-04 10:21 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev, syzkaller-bugs, syzbot+0165480d4ef07360eeda
In-Reply-To: <20190702104600.9744-1-fw@strlen.de>
On Tue, Jul 02, 2019 at 12:46:00PM +0200, Florian Westphal wrote:
> syzbot reported following spat:
>
> BUG: KASAN: use-after-free in __write_once_size include/linux/compiler.h:221
> BUG: KASAN: use-after-free in hlist_del_rcu include/linux/rculist.h:455
> BUG: KASAN: use-after-free in xfrm_hash_rebuild+0xa0d/0x1000 net/xfrm/xfrm_policy.c:1318
> Write of size 8 at addr ffff888095e79c00 by task kworker/1:3/8066
> Workqueue: events xfrm_hash_rebuild
> Call Trace:
> __write_once_size include/linux/compiler.h:221 [inline]
> hlist_del_rcu include/linux/rculist.h:455 [inline]
> xfrm_hash_rebuild+0xa0d/0x1000 net/xfrm/xfrm_policy.c:1318
> process_one_work+0x814/0x1130 kernel/workqueue.c:2269
> Allocated by task 8064:
> __kmalloc+0x23c/0x310 mm/slab.c:3669
> kzalloc include/linux/slab.h:742 [inline]
> xfrm_hash_alloc+0x38/0xe0 net/xfrm/xfrm_hash.c:21
> xfrm_policy_init net/xfrm/xfrm_policy.c:4036 [inline]
> xfrm_net_init+0x269/0xd60 net/xfrm/xfrm_policy.c:4120
> ops_init+0x336/0x420 net/core/net_namespace.c:130
> setup_net+0x212/0x690 net/core/net_namespace.c:316
>
> The faulting address is the address of the old chain head,
> free'd by xfrm_hash_resize().
>
> In xfrm_hash_rehash(), chain heads get re-initialized without
> any hlist_del_rcu:
>
> for (i = hmask; i >= 0; i--)
> INIT_HLIST_HEAD(odst + i);
>
> Then, hlist_del_rcu() gets called on the about to-be-reinserted policy
> when iterating the per-net list of policies.
>
> hlist_del_rcu() will then make chain->first be nonzero again:
>
> static inline void __hlist_del(struct hlist_node *n)
> {
> struct hlist_node *next = n->next; // address of next element in list
> struct hlist_node **pprev = n->pprev;// location of previous elem, this
> // can point at chain->first
> WRITE_ONCE(*pprev, next); // chain->first points to next elem
> if (next)
> next->pprev = pprev;
>
> Then, when we walk chainlist to find insertion point, we may find a
> non-empty list even though we're supposedly reinserting the first
> policy to an empty chain.
>
> To fix this first unlink all exact and inexact policies instead of
> zeroing the list heads.
>
> Add the commands equivalent to the syzbot reproducer to xfrm_policy.sh,
> without fix KASAN catches the corruption as it happens, SLUB poisoning
> detects it a bit later.
>
> Reported-by: syzbot+0165480d4ef07360eeda@syzkaller.appspotmail.com
> Fixes: 1548bc4e0512 ("xfrm: policy: delete inexact policies from inexact list on hash rebuild")
> Signed-off-by: Florian Westphal <fw@strlen.de>
Applied, thanks Florian!
^ permalink raw reply
* Re: [PATCH v2 1/3] vsock/virtio: use RCU to avoid use-after-free on the_virtio_vsock
From: Stefan Hajnoczi @ 2019-07-04 10:17 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Jason Wang, netdev, kvm, virtualization, Michael S. Tsirkin,
David S. Miller, linux-kernel
In-Reply-To: <20190703104135.wg34dobv64k7u4jo@steredhat>
[-- Attachment #1: Type: text/plain, Size: 723 bytes --]
On Wed, Jul 03, 2019 at 12:41:35PM +0200, Stefano Garzarella wrote:
> On Wed, Jul 03, 2019 at 05:53:58PM +0800, Jason Wang wrote:
> > On 2019/6/28 下午8:36, Stefano Garzarella wrote:
> > Another more interesting question, I believe we will do singleton for
> > virtio_vsock structure. Then what's the point of using vdev->priv to access
> > the_virtio_vsock? It looks to me we can it brings extra troubles for doing
> > synchronization.
>
> I thought about it when I tried to use RCU to stop the worker and I
> think make sense. Maybe can be another series after this will be merged.
>
> @Stefan, what do you think about that?
Yes, let's make it a singleton and keep no other references to it.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* RE: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-04 10:13 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Jose Abreu
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, Joao Pinto,
David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai, Ilias Apalodimas
In-Reply-To: <20190704120018.4523a119@carbon>
From: Jesper Dangaard Brouer <brouer@redhat.com>
> The page_pool DMA mapping cannot be "kept" when page traveling into the
> network stack attached to an SKB. (Ilias and I have a long term plan[1]
> to allow this, but you cannot do it ATM).
The reason I recycle the page is this previous call to:
skb_copy_to_linear_data()
So, technically, I'm syncing to CPU the page(s) and then memcpy to a
previously allocated SKB ... So it's safe to just recycle the mapping I
think.
Its kind of using bounce buffers and I do see performance gain in this
(I think the reason is because my setup uses swiotlb for DMA mapping).
Anyway, I'm open to some suggestions on how to improve this ...
> Also remember that the page_pool requires you driver to do the DMA-sync
> operation. I see a dma_sync_single_for_cpu(), but I didn't see a
> dma_sync_single_for_device() (well, I noticed one getting removed).
> (For some HW Ilias tells me that the dma_sync_single_for_device can be
> elided, so maybe this can still be correct for you).
My HW just needs descriptors refilled which are in different coherent
region so I don't see any reason for dma_sync_single_for_device() ...
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jesper Dangaard Brouer @ 2019-07-04 10:00 UTC (permalink / raw)
To: Jose Abreu
Cc: brouer, linux-kernel, netdev, linux-stm32, linux-arm-kernel,
Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai,
Ilias Apalodimas
In-Reply-To: <1b254bb7fc6044c5e6e2fdd9e00088d1d13a808b.1562149883.git.joabreu@synopsys.com>
On Wed, 3 Jul 2019 12:37:50 +0200
Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> @@ -3547,6 +3456,9 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
>
> napi_gro_receive(&ch->rx_napi, skb);
>
> + page_pool_recycle_direct(rx_q->page_pool, buf->page);
This doesn't look correct.
The page_pool DMA mapping cannot be "kept" when page traveling into the
network stack attached to an SKB. (Ilias and I have a long term plan[1]
to allow this, but you cannot do it ATM).
You will have to call:
page_pool_release_page(rx_q->page_pool, buf->page);
This will do a DMA-unmap, and you will likely loose your performance
gain :-(
> + buf->page = NULL;
> +
> priv->dev->stats.rx_packets++;
> priv->dev->stats.rx_bytes += frame_len;
> }
Also remember that the page_pool requires you driver to do the DMA-sync
operation. I see a dma_sync_single_for_cpu(), but I didn't see a
dma_sync_single_for_device() (well, I noticed one getting removed).
(For some HW Ilias tells me that the dma_sync_single_for_device can be
elided, so maybe this can still be correct for you).
[1] https://github.com/xdp-project/xdp-project/blob/master/areas/mem/page_pool02_SKB_return_callback.org
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox