Netdev List
 help / color / mirror / Atom feed
* [PATCH iproute2/net-next v2] tc_util: Add support for showing TCA_STATS_BASIC_HW statistics
From: Eelco Chaudron @ 2018-08-10 11:59 UTC (permalink / raw)
  To: netdev; +Cc: davem, stephen

Add support for showing hardware specific counters to easy
troubleshooting hardware offload.

$ tc -s filter show dev enp3s0np0 parent ffff:
filter protocol ip pref 1 flower chain 0
filter protocol ip pref 1 flower chain 0 handle 0x1
  eth_type ipv4
  dst_ip 2.0.0.0
  src_ip 1.0.0.0
  ip_flags nofrag
  in_hw
        action order 1: mirred (Egress Redirect to device eth1) stolen
        index 1 ref 1 bind 1 installed 0 sec used 0 sec
        Action statistics:
        Sent 534884742 bytes 8915697 pkt (dropped 0, overlimits 0 requeues 0)
        Sent software 187542 bytes 4077 pkt
        Sent hardware 534697200 bytes 8911620 pkt
        backlog 0b 0p requeues 0
        cookie 89173e6a44447001becfd486bda17e29


Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
v2:
 * Removed unnecessary initialization
 * Made not displaying of missing TCA_STATS_BASIC_HW more obvious
 * Use _SL_ macro for single line output

 include/uapi/linux/gen_stats.h |    1 +
 tc/tc_util.c                   |   41 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/include/uapi/linux/gen_stats.h b/include/uapi/linux/gen_stats.h
index 24a861c..065408e 100644
--- a/include/uapi/linux/gen_stats.h
+++ b/include/uapi/linux/gen_stats.h
@@ -12,6 +12,7 @@ enum {
 	TCA_STATS_APP,
 	TCA_STATS_RATE_EST64,
 	TCA_STATS_PAD,
+	TCA_STATS_BASIC_HW,
 	__TCA_STATS_MAX,
 };
 #define TCA_STATS_MAX (__TCA_STATS_MAX - 1)
diff --git a/tc/tc_util.c b/tc/tc_util.c
index d757852..5a1bbf2 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -800,6 +800,44 @@ void print_tm(FILE *f, const struct tcf_t *tm)
 	}
 }
 
+static void print_tcstats_basic_hw(struct rtattr **tbs, char *prefix)
+{
+	struct gnet_stats_basic bs_hw;
+
+	if (!tbs[TCA_STATS_BASIC_HW])
+		return;
+
+	memcpy(&bs_hw, RTA_DATA(tbs[TCA_STATS_BASIC_HW]),
+	       MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC_HW]), sizeof(bs_hw)));
+
+	if (bs_hw.bytes == 0 && bs_hw.packets == 0)
+		return;
+
+	if (tbs[TCA_STATS_BASIC]) {
+		struct gnet_stats_basic bs;
+
+		memcpy(&bs, RTA_DATA(tbs[TCA_STATS_BASIC]),
+		       MIN(RTA_PAYLOAD(tbs[TCA_STATS_BASIC]),
+			   sizeof(bs)));
+
+		if (bs.bytes >= bs_hw.bytes && bs.packets >= bs_hw.packets) {
+			print_string(PRINT_FP, NULL, "%s", _SL_);
+			print_string(PRINT_FP, NULL, "%s", prefix);
+			print_lluint(PRINT_ANY, "sw_bytes",
+				     "Sent software %llu bytes",
+				     bs.bytes - bs_hw.bytes);
+			print_uint(PRINT_ANY, "sw_packets", " %u pkt",
+				   bs.packets - bs_hw.packets);
+		}
+	}
+
+	print_string(PRINT_FP, NULL, "%s", _SL_);
+	print_string(PRINT_FP, NULL, "%s", prefix);
+	print_lluint(PRINT_ANY, "hw_bytes", "Sent hardware %llu bytes",
+		     bs_hw.bytes);
+	print_uint(PRINT_ANY, "hw_packets", " %u pkt", bs_hw.packets);
+}
+
 void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct rtattr **xstats)
 {
 	SPRINT_BUF(b1);
@@ -826,6 +864,9 @@ void print_tcstats2_attr(FILE *fp, struct rtattr *rta, char *prefix, struct rtat
 		print_uint(PRINT_ANY, "requeues", " requeues %u) ", q.requeues);
 	}
 
+	if (tbs[TCA_STATS_BASIC_HW])
+		print_tcstats_basic_hw(tbs, prefix);
+
 	if (tbs[TCA_STATS_RATE_EST64]) {
 		struct gnet_stats_rate_est64 re = {0};
 

^ permalink raw reply related

* Re: [BUG] bpf: syscall: a possible sleep-in-atomic-context bug in map_update_elem()
From: Daniel Borkmann @ 2018-08-10 14:22 UTC (permalink / raw)
  To: Jia-Ju Bai, ast; +Cc: netdev, Linux Kernel Mailing List, brouer
In-Reply-To: <65830741-bf35-4d32-e365-c32fc17c25cb@gmail.com>

On 08/10/2018 04:07 PM, Jia-Ju Bai wrote:
> The kernel may sleep with holding a rcu read lock.
> 
> The function call paths (from bottom to top) in Linux-4.16 are:
> 
> [FUNC] kmalloc(GFP_KERNEL)
> kernel/kthread.c, 283: kmalloc in __kthread_create_on_node
> kernel/kthread.c, 365: __kthread_create_on_node in kthread_create_on_node
> kernel/bpf/cpumap.c, 368: kthread_create_on_node in __cpu_map_entry_alloc
> kernel/bpf/cpumap.c, 490: __cpu_map_entry_alloc in cpu_map_update_elem
> kernel/bpf/syscall.c, 724: [FUNC_PTR]cpu_map_update_elem in map_update_elem
> kernel/bpf/syscall.c, 723: rcu_read_lock in map_update_elem
> 
> Note that [FUNC_PTR] means a function pointer call is used.
> 
> I do not find a good way to fix it, so I only report.
> This is found by my static analysis tool (DSAC).

Thanks for the report Jia-Ju! In the map_update_elem() from syscall
path there's a check map->map_type == BPF_MAP_TYPE_CPUMAP, where we
call the cpumap's map->ops->map_update_elem() while /not/ being under
rcu_read_lock() as in other cases, so looks okay to me. Could you point
out the case for being under rcu_read_lock() more specifically which
the tool found?

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: William Tu @ 2018-08-10 14:11 UTC (permalink / raw)
  To: Stefano Brivio
  Cc: <dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org>,
	Justin Pettit, netdev, Jiri Benc, Joe Stringer
In-Reply-To: <20180804024324.4d900b5e@epycfail>

On Fri, Aug 3, 2018 at 5:43 PM, Stefano Brivio <sbrivio-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> On Fri, 3 Aug 2018 16:01:08 -0700
> Ben Pfaff <blp-LZ6Gd1LRuIk@public.gmane.org> wrote:
>
> > I think that a simple mechanism for fairness is fine.  The direction
> > of extensibility that makes me anxious is how to decide what matters
> > for fairness.  So far, we've talked about per-vport fairness.  That
> > works pretty well for packets coming in from virtual interfaces where
> > each vport represents a separate VM.
>
> Yes, right, that's the case where we have significant issues currently.
>
> > It does not work well if the traffic filling your queues all comes
> > from a single physical port because some source of traffic is sending
> > traffic at a high rate.  In that case, you'll do a lot better if you
> > do fairness based on the source 5-tuple. But if you're doing network
> > virtualization, then the outer source 5-tuples won't necessarily vary
> > much and you'd be better off looking at the VNI and maybe some Geneve
> > TLV options and maybe the inner 5-tuple...
>
> Sure, I see what you mean now. That looks entirely doable if we
> abstract the round-robin bucket selection out of the current patch.
>
> > I would be very pleased if we could integrate a simple mechanism for
> > fairness, based for now on some simple criteria like the source port,
> > but thinking ahead to how we could later make it gracefully extensible
> > to consider more general and possibly customizable criteria.
>
> We could change the patch so that instead of just using the vport for
> round-robin queue insertion, we generalise that and use "buckets"
> instead of vports, and have a set of possible functions that are called
> instead of using port_no directly in ovs_dp_upcall_queue_roundrobin(),
> making this configurable via netlink, per datapath.
>
> We could implement selection based on source port or a hash on the
> source 5-tuple, and the relevant bits of
> ovs_dp_upcall_queue_roundrobin() would look like this:
>
> static int ovs_dp_upcall_queue_roundrobin(struct datapath *dp,
>                                           struct dp_upcall_info *upcall)
> {
>
> [...]
>
>         list_for_each_entry(pos, head, list) {
>                 int bucket = dp->rr_select(pos);
>
>                 /* Count per-bucket upcalls. */
>                 if (dp->upcalls.count[bucket] == U8_MAX) {
>                         err = -ENOSPC;
>                         goto out_clear;
>                 }
>                 dp->upcalls.count[bucket]++;
>
>                 if (bucket == upcall->bucket) {
>                         /* Another upcall for the same bucket: move
> insertion
>                          * point here, keep looking for insertion
> condition to
>                          * be still met further on.
>                          */
>                         find_next = true;
>                         here = pos;
>                         continue;
>                 }
>
>                 count = dp->upcalls.count[bucket];
>                 if (find_next && dp->upcalls.count[bucket] >= count) {
>                         /* Insertion condition met: no need to look
> further,
>                          * unless another upcall for the same port occurs
> later.
>                          */
>                         find_next = false;
>                         here = pos;
>                 }
>         }
>
> [...]
>
> }
>
> and implementations for dp->rr_select() would look like:
>
> int rr_select_vport(struct dp_upcall_info *upcall)
> {
>         return upcall->port_no;
> }
>
> int rr_select_srcport(struct dp_upcall_info *upcall)
> {
>         /* look up source port from upcall->skb... */
> }
>
> And we could then easily extend this to use BPF with maps one day.
>
>
Hi Stefano,

If you want to experiment with BPF, Joe and I have some prototype.
We implemented the upcall mechanism using BPF perf event helper function
https://github.com/williamtu/ovs-ebpf/blob/master/bpf/datapath.c#L62

And there are threads polling the perf ring buffer to receive packets from
BPF.
https://github.com/williamtu/ovs-ebpf/blob/master/lib/perf-event.c#L232

If I follow the discussion correctly, before upcall, you need to queue
packets based
on different configurations (vport/hash/vni/5-tuple/...) and queue to
different buckets
when congestion happens. In this case, you probably needs a BPF map to
enqueue/dequeue
the packet. BPF queue map is not supported yet, but there is patch
available:
[iovisor-dev] [RFC PATCH 1/3] bpf: add bpf queue map

So how to enqueue and dequeue packets depends on user's BPF implementation.
This allows fairness scheme to be extensible.

Regards,
William



This is for clarity by the way, but I guess we should avoid indirect
> calls in the final implementation.
>
> What do you think?
>
> --
> Stefano
>

^ permalink raw reply

* [BUG] bpf: syscall: a possible sleep-in-atomic-context bug in map_update_elem()
From: Jia-Ju Bai @ 2018-08-10 14:07 UTC (permalink / raw)
  To: ast, daniel; +Cc: netdev, Linux Kernel Mailing List

The kernel may sleep with holding a rcu read lock.

The function call paths (from bottom to top) in Linux-4.16 are:

[FUNC] kmalloc(GFP_KERNEL)
kernel/kthread.c, 283: kmalloc in __kthread_create_on_node
kernel/kthread.c, 365: __kthread_create_on_node in kthread_create_on_node
kernel/bpf/cpumap.c, 368: kthread_create_on_node in __cpu_map_entry_alloc
kernel/bpf/cpumap.c, 490: __cpu_map_entry_alloc in cpu_map_update_elem
kernel/bpf/syscall.c, 724: [FUNC_PTR]cpu_map_update_elem in map_update_elem
kernel/bpf/syscall.c, 723: rcu_read_lock in map_update_elem

Note that [FUNC_PTR] means a function pointer call is used.

I do not find a good way to fix it, so I only report.
This is found by my static analysis tool (DSAC).


Best wishes,
Jia-Ju Bai

^ permalink raw reply

* Re: [PATCH lora-next 01/10] net: lora: sx1301: add register, bit-fields, and helpers for regmap
From: Andreas Färber @ 2018-08-10 11:37 UTC (permalink / raw)
  To: Ben Whitten, Ben Whitten
  Cc: starnight@g.ncu.edu.tw, hasnain.virk@arm.com,
	netdev@vger.kernel.org, Xue Liu, Sebastian Heß, Yannick Lanz
In-Reply-To: <BY1PR02MB111463B66B79C5008905E011E7250@BY1PR02MB1114.namprd02.prod.outlook.com>

Am 09.08.2018 um 15:23 schrieb Ben Whitten:
>> BTW we'll need this regmap conversion for the picoGW_hal,
>> so once we
>> have a working SPI regmap driver, we'll need to split out the
>> SPI bits,
>> similar to sx125x.
> 
> I am unfamiliar with the picoGW_hal, do they expose the sx1301
> as a device on a regmap_bus then?

It uses an MCU implementing a USB CDC interface that implements UART
commands corresponding to the individual SPI read and write modes like
single and burst. That reference design is for SX1308, but some vendors
appear to be using it for SX1301, too.

https://github.com/Lora-net/picoGW_mcu

So I believe I'll need to implement a regmap_bus in a new serdev driver
to support it, once I gain access to such an implementation.

But there's other general serdev problems with USB not limited to picoGW
that I'll need to start a thread about on linux-serial/linux-usb...

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply

* REPLY AS SOON AS POSSIBLE
From: Dr Chien Direktor von Hang Seng @ 2018-08-10  6:42 UTC (permalink / raw)
  To: Recipients

I am Vice Chairman of Hang Seng Bank, I have Important Matter to Discuss with you concerning my late client. Died without a NEXT OF KIN. Send me your private email for full details information. email me at E-Mail: draymdm@gmail.com

Regards 
Mr.Fung

^ permalink raw reply

* Re: [Query]: DSA Understanding
From: Lad, Prabhakar @ 2018-08-10 11:26 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev
In-Reply-To: <20180809172334.GA25449@lunn.ch>

Hi Andrew,

On Thu, Aug 9, 2018 at 6:23 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Its coming from the switch lan4 I have attached the png, where
> > C4:F3:12:08:FE:7F is
> > the mac of lan4, which is broadcast to ff:ff:ff:ff:ff:ff, which is
> > causing rx counter on
> > PC to go up.
>
> So, big packets are making it from the switch to the PC. But the small
> ARP packets are not.
>
> This is what Florian was suggesting.
>
> ARP packets are smaller than 64 bytes, which is the minimum packet
> size for Ethernet. Any packets smaller than 64 bytes are called runt
> packets. They have to be padded upto 64 bytes in order to make them
> valid. Otherwise the destination, or any switch along the path, might
> throw them away.
>
> What could be happening is that the CSPW driver or hardware is padding
> the packet to 64 bytes. But that packet has a DSA header in it. The
> switch removes the header, recalculate the checksum and sends the
> packet. It is now either 4 or 8 bytes smaller, depending on what DSA
> header was used. It then becomes a runt packet.
>
Thank you for the clarification, this really helped me out.

> Florian had to fix this problem recently.
>
> http://patchwork.ozlabs.org/patch/836534/
>
But seems like this patch was never accepted, instead
brcm_tag_xmit_ll() does it if I am understanding it correctly.
similarly to this ksz_xmit() is taking care of padding.

> You probably need something similar for the cpsw.
>
looking at the tag_ksz.c in xmit function this is taken care of

/* For Ingress (Host -> KSZ), 2 bytes are added before FCS.
 * ---------------------------------------------------------------------------
 * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|tag1(1byte)|FCS(4bytes)
 * ---------------------------------------------------------------------------
 * tag0 : Prioritization (not used now)
 * tag1 : each bit represents port (eg, 0x01=port1, 0x02=port2, 0x10=port5)
 *
 * For Egress (KSZ -> Host), 1 byte is added before FCS.
 * ---------------------------------------------------------------------------
 * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
 * ---------------------------------------------------------------------------
 * tag0 : zero-based value represents port
 *      (eg, 0x00=port1, 0x02=port3, 0x06=port7)
 */

#define    KSZ_INGRESS_TAG_LEN    2
#define    KSZ_EGRESS_TAG_LEN    1

static struct sk_buff *ksz_xmit(struct sk_buff *skb, struct net_device *dev)
{
    struct dsa_slave_priv *p = netdev_priv(dev);
    struct sk_buff *nskb;
    int padlen;
    u8 *tag;

    padlen = (skb->len >= ETH_ZLEN) ? 0 : ETH_ZLEN - skb->len;

    if (skb_tailroom(skb) >= padlen + KSZ_INGRESS_TAG_LEN) {
        /* Let dsa_slave_xmit() free skb */
        if (__skb_put_padto(skb, skb->len + padlen, false))
            return NULL;

        nskb = skb;
    } else {
        nskb = alloc_skb(NET_IP_ALIGN + skb->len +
                 padlen + KSZ_INGRESS_TAG_LEN, GFP_ATOMIC);
        if (!nskb)
            return NULL;
        skb_reserve(nskb, NET_IP_ALIGN);

        skb_reset_mac_header(nskb);
        skb_set_network_header(nskb,
                       skb_network_header(skb) - skb->head);
        skb_set_transport_header(nskb,
                     skb_transport_header(skb) - skb->head);
        skb_copy_and_csum_dev(skb, skb_put(nskb, skb->len));

        /* Let skb_put_padto() free nskb, and let dsa_slave_xmit() free
         * skb
         */
        if (skb_put_padto(nskb, nskb->len + padlen))
            return NULL;

        consume_skb(skb);
    }

    tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
    tag[0] = 0;
    tag[1] = 1 << p->dp->index; /* destination port */

    return nskb;
}

Cheers,
--Prabhakar Lad

^ permalink raw reply

* [PATCH net-next 7/8] l2tp: zero out stats in pppol2tp_copy_stats()
From: Guillaume Nault @ 2018-08-10 11:22 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

Integrate memset(0) in pppol2tp_copy_stats() to avoid calling it
manually every time.

While there, constify 'stats'.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 net/l2tp/l2tp_ppp.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index bdfbd3ed7e14..e2eea60bf875 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1026,8 +1026,10 @@ static int pppol2tp_getname(struct socket *sock, struct sockaddr *uaddr,
  ****************************************************************************/
 
 static void pppol2tp_copy_stats(struct pppol2tp_ioc_stats *dest,
-				struct l2tp_stats *stats)
+				const struct l2tp_stats *stats)
 {
+	memset(dest, 0, sizeof(*dest));
+
 	dest->tx_packets = atomic_long_read(&stats->tx_packets);
 	dest->tx_bytes = atomic_long_read(&stats->tx_bytes);
 	dest->tx_errors = atomic_long_read(&stats->tx_errors);
@@ -1044,7 +1046,6 @@ static int pppol2tp_tunnel_copy_stats(struct pppol2tp_ioc_stats *stats,
 	struct l2tp_session *session;
 
 	if (!stats->session_id) {
-		memset(stats, 0, sizeof(*stats));
 		pppol2tp_copy_stats(stats, &tunnel->stats);
 		return 0;
 	}
@@ -1061,7 +1062,6 @@ static int pppol2tp_tunnel_copy_stats(struct pppol2tp_ioc_stats *stats,
 		return -EBADR;
 	}
 
-	memset(stats, 0, sizeof(*stats));
 	pppol2tp_copy_stats(stats, &session->stats);
 	l2tp_session_dec_refcount(session);
 
@@ -1126,7 +1126,6 @@ static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 
 			stats.session_id = session_id;
 		} else {
-			memset(&stats, 0, sizeof(stats));
 			pppol2tp_copy_stats(&stats, &session->stats);
 			stats.session_id = session->session_id;
 		}
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 8/8] l2tp: let pppol2tp_ioctl() fallback to dev_ioctl()
From: Guillaume Nault @ 2018-08-10 11:22 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

Return -ENOIOCTLCMD for unknown ioctl commands. This lets dev_ioctl()
handle generic socket ioctls like SIOCGIFNAME or SIOCGIFINDEX.
PF_PPPOX/PX_PROTO_OL2TP was one of the few socket types not honouring
this mechanism.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 net/l2tp/l2tp_ppp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index e2eea60bf875..62f2d3f1e431 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1137,7 +1137,7 @@ static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 		break;
 
 	default:
-		return -ENOSYS;
+		return -ENOIOCTLCMD;
 	}
 
 	return 0;
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 6/8] l2tp: remove pppol2tp_session_ioctl()
From: Guillaume Nault @ 2018-08-10 11:22 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

pppol2tp_ioctl() has everything in place for handling PPPIOCGL2TPSTATS
on session sockets. We just need to copy the stats and set ->session_id.

As a side effect of sharing session and tunnel code, ->using_ipsec is
properly set even when the request was made using a session socket.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 include/uapi/linux/ppp-ioctl.h |  2 +-
 net/l2tp/l2tp_ppp.c            | 50 ++--------------------------------
 2 files changed, 4 insertions(+), 48 deletions(-)

diff --git a/include/uapi/linux/ppp-ioctl.h b/include/uapi/linux/ppp-ioctl.h
index 784c2e3e572e..88b5f9990320 100644
--- a/include/uapi/linux/ppp-ioctl.h
+++ b/include/uapi/linux/ppp-ioctl.h
@@ -68,7 +68,7 @@ struct ppp_option_data {
 struct pppol2tp_ioc_stats {
 	__u16		tunnel_id;	/* redundant */
 	__u16		session_id;	/* if zero, get tunnel stats */
-	__u32		using_ipsec:1;	/* valid only for session_id == 0 */
+	__u32		using_ipsec:1;
 	__aligned_u64	tx_packets;
 	__aligned_u64	tx_bytes;
 	__aligned_u64	tx_errors;
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 2afd3ab8a551..bdfbd3ed7e14 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1068,52 +1068,6 @@ static int pppol2tp_tunnel_copy_stats(struct pppol2tp_ioc_stats *stats,
 	return 0;
 }
 
-/* Session ioctl helper.
- */
-static int pppol2tp_session_ioctl(struct l2tp_session *session,
-				  unsigned int cmd, unsigned long arg)
-{
-	int err = 0;
-	struct sock *sk;
-	struct l2tp_tunnel *tunnel = session->tunnel;
-	struct pppol2tp_ioc_stats stats;
-
-	l2tp_dbg(session, L2TP_MSG_CONTROL,
-		 "%s: pppol2tp_session_ioctl(cmd=%#x, arg=%#lx)\n",
-		 session->name, cmd, arg);
-
-	sk = pppol2tp_session_get_sock(session);
-	if (!sk)
-		return -EBADR;
-
-	switch (cmd) {
-	case PPPIOCGL2TPSTATS:
-		err = -ENXIO;
-		if (!(sk->sk_state & PPPOX_CONNECTED))
-			break;
-
-		memset(&stats, 0, sizeof(stats));
-		stats.tunnel_id = tunnel->tunnel_id;
-		stats.session_id = session->session_id;
-		pppol2tp_copy_stats(&stats, &session->stats);
-		if (copy_to_user((void __user *) arg, &stats,
-				 sizeof(stats)))
-			break;
-		l2tp_info(session, L2TP_MSG_CONTROL, "%s: get L2TP stats\n",
-			  session->name);
-		err = 0;
-		break;
-
-	default:
-		err = -ENOSYS;
-		break;
-	}
-
-	sock_put(sk);
-
-	return err;
-}
-
 static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 			  unsigned long arg)
 {
@@ -1172,7 +1126,9 @@ static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 
 			stats.session_id = session_id;
 		} else {
-			return pppol2tp_session_ioctl(session, cmd, arg);
+			memset(&stats, 0, sizeof(stats));
+			pppol2tp_copy_stats(&stats, &session->stats);
+			stats.session_id = session->session_id;
 		}
 		stats.tunnel_id = session->tunnel->tunnel_id;
 		stats.using_ipsec = l2tp_tunnel_uses_xfrm(session->tunnel);
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 5/8] l2tp: remove pppol2tp_tunnel_ioctl()
From: Guillaume Nault @ 2018-08-10 11:22 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

Handle PPPIOCGL2TPSTATS in pppol2tp_ioctl() if the socket represents a
tunnel. This one is a bit special because the caller may use the tunnel
socket to retrieve statistics of one of its sessions. If the session_id
is set, the corresponding session's statistics are returned, instead of
those of the tunnel. This is handled by the new
pppol2tp_tunnel_copy_stats() helper function.

Set ->tunnel_id and ->using_ipsec out of the conditional, so
that it can be used by the 'else' branch in the following patch.
We cannot do that for ->session_id, because tunnel sockets have to
report the value that was originally passed in 'stats.session_id',
while session sockets have to report their own session_id.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 net/l2tp/l2tp_ppp.c | 132 ++++++++++++++++++--------------------------
 1 file changed, 53 insertions(+), 79 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index f4ec6b2a093e..2afd3ab8a551 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1038,6 +1038,36 @@ static void pppol2tp_copy_stats(struct pppol2tp_ioc_stats *dest,
 	dest->rx_errors = atomic_long_read(&stats->rx_errors);
 }
 
+static int pppol2tp_tunnel_copy_stats(struct pppol2tp_ioc_stats *stats,
+				      struct l2tp_tunnel *tunnel)
+{
+	struct l2tp_session *session;
+
+	if (!stats->session_id) {
+		memset(stats, 0, sizeof(*stats));
+		pppol2tp_copy_stats(stats, &tunnel->stats);
+		return 0;
+	}
+
+	/* If session_id is set, search the corresponding session in the
+	 * context of this tunnel and record the session's statistics.
+	 */
+	session = l2tp_tunnel_get_session(tunnel, stats->session_id);
+	if (!session)
+		return -EBADR;
+
+	if (session->pwtype != L2TP_PWTYPE_PPP) {
+		l2tp_session_dec_refcount(session);
+		return -EBADR;
+	}
+
+	memset(stats, 0, sizeof(*stats));
+	pppol2tp_copy_stats(stats, &session->stats);
+	l2tp_session_dec_refcount(session);
+
+	return 0;
+}
+
 /* Session ioctl helper.
  */
 static int pppol2tp_session_ioctl(struct l2tp_session *session,
@@ -1084,84 +1114,10 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 	return err;
 }
 
-/* Tunnel ioctl helper.
- *
- * Note the special handling for PPPIOCGL2TPSTATS below. If the ioctl data
- * specifies a session_id, the session ioctl handler is called. This allows an
- * application to retrieve session stats via a tunnel socket.
- */
-static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel,
-				 unsigned int cmd, unsigned long arg)
-{
-	int err = 0;
-	struct sock *sk;
-	struct pppol2tp_ioc_stats stats;
-
-	l2tp_dbg(tunnel, L2TP_MSG_CONTROL,
-		 "%s: pppol2tp_tunnel_ioctl(cmd=%#x, arg=%#lx)\n",
-		 tunnel->name, cmd, arg);
-
-	sk = tunnel->sock;
-	sock_hold(sk);
-
-	switch (cmd) {
-	case PPPIOCGL2TPSTATS:
-		err = -ENXIO;
-		if (!(sk->sk_state & PPPOX_CONNECTED))
-			break;
-
-		if (copy_from_user(&stats, (void __user *) arg,
-				   sizeof(stats))) {
-			err = -EFAULT;
-			break;
-		}
-		if (stats.session_id != 0) {
-			/* resend to session ioctl handler */
-			struct l2tp_session *session;
-
-			session = l2tp_tunnel_get_session(tunnel,
-							  stats.session_id);
-			if (!session) {
-				err = -EBADR;
-				break;
-			}
-			if (session->pwtype != L2TP_PWTYPE_PPP) {
-				l2tp_session_dec_refcount(session);
-				err = -EBADR;
-				break;
-			}
-
-			err = pppol2tp_session_ioctl(session, cmd, arg);
-			l2tp_session_dec_refcount(session);
-			break;
-		}
-		stats.using_ipsec = l2tp_tunnel_uses_xfrm(tunnel);
-		pppol2tp_copy_stats(&stats, &tunnel->stats);
-		if (copy_to_user((void __user *) arg, &stats, sizeof(stats))) {
-			err = -EFAULT;
-			break;
-		}
-		l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: get L2TP stats\n",
-			  tunnel->name);
-		err = 0;
-		break;
-
-	default:
-		err = -ENOSYS;
-		break;
-	}
-
-	sock_put(sk);
-
-	return err;
-}
-
-/* Main ioctl() handler.
- * Dispatch to tunnel or session helpers depending on the socket.
- */
 static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 			  unsigned long arg)
 {
+	struct pppol2tp_ioc_stats stats;
 	struct l2tp_session *session;
 	int val;
 
@@ -1200,11 +1156,29 @@ static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 			return -ENOTCONN;
 
 		/* Session 0 represents the parent tunnel */
-		if (!session->session_id && !session->peer_session_id)
-			return pppol2tp_tunnel_ioctl(session->tunnel, cmd,
-						     arg);
-		else
+		if (!session->session_id && !session->peer_session_id) {
+			u32 session_id;
+			int err;
+
+			if (copy_from_user(&stats, (void __user *)arg,
+					   sizeof(stats)))
+				return -EFAULT;
+
+			session_id = stats.session_id;
+			err = pppol2tp_tunnel_copy_stats(&stats,
+							 session->tunnel);
+			if (err < 0)
+				return err;
+
+			stats.session_id = session_id;
+		} else {
 			return pppol2tp_session_ioctl(session, cmd, arg);
+		}
+		stats.tunnel_id = session->tunnel->tunnel_id;
+		stats.using_ipsec = l2tp_tunnel_uses_xfrm(session->tunnel);
+
+		if (copy_to_user((void __user *)arg, &stats, sizeof(stats)))
+			return -EFAULT;
 		break;
 
 	default:
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 4/8] l2tp: handle PPPIOC[GS]MRU and PPPIOC[GS]FLAGS in pppol2tp_ioctl()
From: Guillaume Nault @ 2018-08-10 11:21 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

Let pppol2tp_ioctl() handle ioctl commands directly. It still relies on
pppol2tp_{session,tunnel}_ioctl() for PPPIOCGL2TPSTATS.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
Checkpatch does not like the -ENOSYS return value, which should only be
used for non-existing syscalls. I have kept them so that userspace gets
the same errors before and after this patch.

However I am feeling that I may be too conservative and that, maybe
the *MRU and *FLAGS commands should be dropped entirely, like *IFMTU
was. That would drop two of the three -ENOSYS errors (the last one is
dropped by the last patch anyway).

David, James, do you have any preference? After all, the MRU/FLAGS
ioctls never had any effect on pppol2tp sockets. So I do not expect
code to use them or expect them to succeed. I you are ok, I will send
a followup patch for dropping those commands.

 net/l2tp/l2tp_ppp.c | 73 +++++++++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 29 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index e3ed8d473d91..f4ec6b2a093e 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1045,7 +1045,6 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 {
 	int err = 0;
 	struct sock *sk;
-	int val = (int) arg;
 	struct l2tp_tunnel *tunnel = session->tunnel;
 	struct pppol2tp_ioc_stats stats;
 
@@ -1058,22 +1057,6 @@ static int pppol2tp_session_ioctl(struct l2tp_session *session,
 		return -EBADR;
 
 	switch (cmd) {
-	case PPPIOCGMRU:
-	case PPPIOCGFLAGS:
-		err = -EFAULT;
-		if (put_user(0, (int __user *)arg))
-			break;
-		err = 0;
-		break;
-
-	case PPPIOCSMRU:
-	case PPPIOCSFLAGS:
-		err = -EFAULT;
-		if (get_user(val, (int __user *)arg))
-			break;
-		err = 0;
-		break;
-
 	case PPPIOCGL2TPSTATS:
 		err = -ENXIO;
 		if (!(sk->sk_state & PPPOX_CONNECTED))
@@ -1180,23 +1163,55 @@ static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 			  unsigned long arg)
 {
 	struct l2tp_session *session;
-	struct l2tp_tunnel *tunnel;
+	int val;
+
+	switch (cmd) {
+	case PPPIOCGMRU:
+	case PPPIOCGFLAGS:
+		session = sock->sk->sk_user_data;
+		if (!session)
+			return -ENOTCONN;
 
-	session = sock->sk->sk_user_data;
-	if (!session)
-		return -ENOTCONN;
+		/* Not defined for tunnels */
+		if (!session->session_id && !session->peer_session_id)
+			return -ENOSYS;
 
-	/* Special case: if session's session_id is zero, treat ioctl as a
-	 * tunnel ioctl
-	 */
-	if ((session->session_id == 0) &&
-	    (session->peer_session_id == 0)) {
-		tunnel = session->tunnel;
+		if (put_user(0, (int __user *)arg))
+			return -EFAULT;
+		break;
+
+	case PPPIOCSMRU:
+	case PPPIOCSFLAGS:
+		session = sock->sk->sk_user_data;
+		if (!session)
+			return -ENOTCONN;
 
-		return pppol2tp_tunnel_ioctl(tunnel, cmd, arg);
+		/* Not defined for tunnels */
+		if (!session->session_id && !session->peer_session_id)
+			return -ENOSYS;
+
+		if (get_user(val, (int __user *)arg))
+			return -EFAULT;
+		break;
+
+	case PPPIOCGL2TPSTATS:
+		session = sock->sk->sk_user_data;
+		if (!session)
+			return -ENOTCONN;
+
+		/* Session 0 represents the parent tunnel */
+		if (!session->session_id && !session->peer_session_id)
+			return pppol2tp_tunnel_ioctl(session->tunnel, cmd,
+						     arg);
+		else
+			return pppol2tp_session_ioctl(session, cmd, arg);
+		break;
+
+	default:
+		return -ENOSYS;
 	}
 
-	return pppol2tp_session_ioctl(session, cmd, arg);
+	return 0;
 }
 
 /*****************************************************************************
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 3/8] l2tp: simplify pppol2tp_ioctl()
From: Guillaume Nault @ 2018-08-10 11:21 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

  * Drop test on 'sk': sock->sk cannot be NULL, or pppox_ioctl() could
    not have called us.

  * Drop test on 'SOCK_DEAD' state: if this flag was set, the socket
    would be in the process of being released and no ioctl could be
    running anymore.

  * Drop test on 'PPPOX_*' state: we depend on ->sk_user_data to get
    the session structure. If it is non-NULL, then the socket is
    connected. Testing for PPPOX_* is redundant.

  * Retrieve session using ->sk_user_data directly, instead of going
    through pppol2tp_sock_to_session(). This avoids grabbing a useless
    reference on the socket.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 net/l2tp/l2tp_ppp.c | 33 ++++++---------------------------
 1 file changed, 6 insertions(+), 27 deletions(-)

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index cd43d02484e4..e3ed8d473d91 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1179,28 +1179,12 @@ static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel,
 static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 			  unsigned long arg)
 {
-	struct sock *sk = sock->sk;
 	struct l2tp_session *session;
 	struct l2tp_tunnel *tunnel;
-	int err;
-
-	if (!sk)
-		return 0;
-
-	err = -EBADF;
-	if (sock_flag(sk, SOCK_DEAD) != 0)
-		goto end;
-
-	err = -ENOTCONN;
-	if ((sk->sk_user_data == NULL) ||
-	    (!(sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND))))
-		goto end;
 
-	/* Get session context from the socket */
-	err = -EBADF;
-	session = pppol2tp_sock_to_session(sk);
-	if (session == NULL)
-		goto end;
+	session = sock->sk->sk_user_data;
+	if (!session)
+		return -ENOTCONN;
 
 	/* Special case: if session's session_id is zero, treat ioctl as a
 	 * tunnel ioctl
@@ -1208,16 +1192,11 @@ static int pppol2tp_ioctl(struct socket *sock, unsigned int cmd,
 	if ((session->session_id == 0) &&
 	    (session->peer_session_id == 0)) {
 		tunnel = session->tunnel;
-		err = pppol2tp_tunnel_ioctl(tunnel, cmd, arg);
-		goto end_put_sess;
-	}
 
-	err = pppol2tp_session_ioctl(session, cmd, arg);
+		return pppol2tp_tunnel_ioctl(tunnel, cmd, arg);
+	}
 
-end_put_sess:
-	sock_put(sk);
-end:
-	return err;
+	return pppol2tp_session_ioctl(session, cmd, arg);
 }
 
 /*****************************************************************************
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 2/8] l2tp: split l2tp_session_get()
From: Guillaume Nault @ 2018-08-10 11:21 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

l2tp_session_get() is used for two different purposes. If 'tunnel' is
NULL, the session is searched globally in the supplied network
namespace. Otherwise it is searched exclusively in the tunnel context.

Callers always know the context in which they need to search the
session. But some of them do provide both a namespace and a tunnel,
making the semantic of the call unclear.

This patch defines l2tp_tunnel_get_session() for lookups done in a
tunnel and restricts l2tp_session_get() to namespace searches.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 net/l2tp/l2tp_core.c    | 50 ++++++++++++++++++++---------------------
 net/l2tp/l2tp_core.h    |  6 ++---
 net/l2tp/l2tp_ip.c      |  2 +-
 net/l2tp/l2tp_ip6.c     |  2 +-
 net/l2tp/l2tp_netlink.c |  4 ++--
 net/l2tp/l2tp_ppp.c     |  8 +++----
 6 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index ac6a00bcec71..2bd701a58aa6 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -203,44 +203,44 @@ struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth)
 }
 EXPORT_SYMBOL_GPL(l2tp_tunnel_get_nth);
 
-/* Lookup a session. A new reference is held on the returned session. */
-struct l2tp_session *l2tp_session_get(const struct net *net,
-				      struct l2tp_tunnel *tunnel,
-				      u32 session_id)
+struct l2tp_session *l2tp_tunnel_get_session(struct l2tp_tunnel *tunnel,
+					     u32 session_id)
 {
 	struct hlist_head *session_list;
 	struct l2tp_session *session;
 
-	if (!tunnel) {
-		struct l2tp_net *pn = l2tp_pernet(net);
-
-		session_list = l2tp_session_id_hash_2(pn, session_id);
+	session_list = l2tp_session_id_hash(tunnel, session_id);
 
-		rcu_read_lock_bh();
-		hlist_for_each_entry_rcu(session, session_list, global_hlist) {
-			if (session->session_id == session_id) {
-				l2tp_session_inc_refcount(session);
-				rcu_read_unlock_bh();
+	read_lock_bh(&tunnel->hlist_lock);
+	hlist_for_each_entry(session, session_list, hlist)
+		if (session->session_id == session_id) {
+			l2tp_session_inc_refcount(session);
+			read_unlock_bh(&tunnel->hlist_lock);
 
-				return session;
-			}
+			return session;
 		}
-		rcu_read_unlock_bh();
+	read_unlock_bh(&tunnel->hlist_lock);
 
-		return NULL;
-	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(l2tp_tunnel_get_session);
 
-	session_list = l2tp_session_id_hash(tunnel, session_id);
-	read_lock_bh(&tunnel->hlist_lock);
-	hlist_for_each_entry(session, session_list, hlist) {
+struct l2tp_session *l2tp_session_get(const struct net *net, u32 session_id)
+{
+	struct hlist_head *session_list;
+	struct l2tp_session *session;
+
+	session_list = l2tp_session_id_hash_2(l2tp_pernet(net), session_id);
+
+	rcu_read_lock_bh();
+	hlist_for_each_entry_rcu(session, session_list, global_hlist)
 		if (session->session_id == session_id) {
 			l2tp_session_inc_refcount(session);
-			read_unlock_bh(&tunnel->hlist_lock);
+			rcu_read_unlock_bh();
 
 			return session;
 		}
-	}
-	read_unlock_bh(&tunnel->hlist_lock);
+	rcu_read_unlock_bh();
 
 	return NULL;
 }
@@ -872,7 +872,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
 	}
 
 	/* Find the session context */
-	session = l2tp_session_get(tunnel->l2tp_net, tunnel, session_id);
+	session = l2tp_tunnel_get_session(tunnel, session_id);
 	if (!session || !session->recv_skb) {
 		if (session)
 			l2tp_session_dec_refcount(session);
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 04a9488c54b4..8480a0af973e 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -196,12 +196,12 @@ static inline void *l2tp_session_priv(struct l2tp_session *session)
 
 struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);
 struct l2tp_tunnel *l2tp_tunnel_get_nth(const struct net *net, int nth);
+struct l2tp_session *l2tp_tunnel_get_session(struct l2tp_tunnel *tunnel,
+					     u32 session_id);
 
 void l2tp_tunnel_free(struct l2tp_tunnel *tunnel);
 
-struct l2tp_session *l2tp_session_get(const struct net *net,
-				      struct l2tp_tunnel *tunnel,
-				      u32 session_id);
+struct l2tp_session *l2tp_session_get(const struct net *net, u32 session_id);
 struct l2tp_session *l2tp_session_get_nth(struct l2tp_tunnel *tunnel, int nth);
 struct l2tp_session *l2tp_session_get_by_ifname(const struct net *net,
 						const char *ifname);
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index 0bc39cc20a3f..35f6f86d4dcc 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -144,7 +144,7 @@ static int l2tp_ip_recv(struct sk_buff *skb)
 	}
 
 	/* Ok, this is a data packet. Lookup the session. */
-	session = l2tp_session_get(net, NULL, session_id);
+	session = l2tp_session_get(net, session_id);
 	if (!session)
 		goto discard;
 
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 42f828cf62fb..237f1a4a0b0c 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -157,7 +157,7 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
 	}
 
 	/* Ok, this is a data packet. Lookup the session. */
-	session = l2tp_session_get(net, NULL, session_id);
+	session = l2tp_session_get(net, session_id);
 	if (!session)
 		goto discard;
 
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 357503e5acd5..edbd5d1fbcde 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -66,7 +66,7 @@ static struct l2tp_session *l2tp_nl_session_get(struct genl_info *info)
 		session_id = nla_get_u32(info->attrs[L2TP_ATTR_SESSION_ID]);
 		tunnel = l2tp_tunnel_get(net, tunnel_id);
 		if (tunnel) {
-			session = l2tp_session_get(net, tunnel, session_id);
+			session = l2tp_tunnel_get_session(tunnel, session_id);
 			l2tp_tunnel_dec_refcount(tunnel);
 		}
 	}
@@ -627,7 +627,7 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
 							   &cfg);
 
 	if (ret >= 0) {
-		session = l2tp_session_get(net, tunnel, session_id);
+		session = l2tp_tunnel_get_session(tunnel, session_id);
 		if (session) {
 			ret = l2tp_session_notify(&l2tp_nl_family, info, session,
 						  L2TP_CMD_SESSION_CREATE);
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index c33ef9a3f3b5..cd43d02484e4 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -757,7 +757,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
 	if (tunnel->peer_tunnel_id == 0)
 		tunnel->peer_tunnel_id = info.peer_tunnel_id;
 
-	session = l2tp_session_get(sock_net(sk), tunnel, info.session_id);
+	session = l2tp_tunnel_get_session(tunnel, info.session_id);
 	if (session) {
 		drop_refcnt = true;
 
@@ -1134,10 +1134,10 @@ static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel,
 		}
 		if (stats.session_id != 0) {
 			/* resend to session ioctl handler */
-			struct l2tp_session *session =
-				l2tp_session_get(sock_net(sk), tunnel,
-						 stats.session_id);
+			struct l2tp_session *session;
 
+			session = l2tp_tunnel_get_session(tunnel,
+							  stats.session_id);
 			if (!session) {
 				err = -EBADR;
 				break;
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 1/8] l2tp: define l2tp_tunnel_uses_xfrm()
From: Guillaume Nault @ 2018-08-10 11:21 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman
In-Reply-To: <cover.1533895306.git.g.nault@alphalink.fr>

Use helper function to figure out if a tunnel is using ipsec.
Also, avoid accessing ->sk_policy directly since it's RCU protected.

Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
 net/l2tp/l2tp_core.h    | 19 +++++++++++++++++++
 net/l2tp/l2tp_netlink.c |  7 +------
 net/l2tp/l2tp_ppp.c     |  5 +----
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 5804065dfbfb..04a9488c54b4 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -15,6 +15,10 @@
 #include <net/dst.h>
 #include <net/sock.h>
 
+#ifdef CONFIG_XFRM
+#include <net/xfrm.h>
+#endif
+
 /* Just some random numbers */
 #define L2TP_TUNNEL_MAGIC	0x42114DDA
 #define L2TP_SESSION_MAGIC	0x0C04EB7D
@@ -284,6 +288,21 @@ static inline u32 l2tp_tunnel_dst_mtu(const struct l2tp_tunnel *tunnel)
 	return mtu;
 }
 
+#ifdef CONFIG_XFRM
+static inline bool l2tp_tunnel_uses_xfrm(const struct l2tp_tunnel *tunnel)
+{
+	struct sock *sk = tunnel->sock;
+
+	return sk && (rcu_access_pointer(sk->sk_policy[0]) ||
+		      rcu_access_pointer(sk->sk_policy[1]));
+}
+#else
+static inline bool l2tp_tunnel_uses_xfrm(const struct l2tp_tunnel *tunnel)
+{
+	return false;
+}
+#endif
+
 #define l2tp_printk(ptr, type, func, fmt, ...)				\
 do {									\
 	if (((ptr)->debug) & (type))					\
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 2e1e92651545..357503e5acd5 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -710,9 +710,6 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
 	void *hdr;
 	struct nlattr *nest;
 	struct l2tp_tunnel *tunnel = session->tunnel;
-	struct sock *sk = NULL;
-
-	sk = tunnel->sock;
 
 	hdr = genlmsg_put(skb, portid, seq, &l2tp_nl_family, flags, cmd);
 	if (!hdr)
@@ -738,10 +735,8 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
 	    nla_put_u8(skb, L2TP_ATTR_RECV_SEQ, session->recv_seq) ||
 	    nla_put_u8(skb, L2TP_ATTR_SEND_SEQ, session->send_seq) ||
 	    nla_put_u8(skb, L2TP_ATTR_LNS_MODE, session->lns_mode) ||
-#ifdef CONFIG_XFRM
-	    (((sk) && (sk->sk_policy[0] || sk->sk_policy[1])) &&
+	    (l2tp_tunnel_uses_xfrm(tunnel) &&
 	     nla_put_u8(skb, L2TP_ATTR_USING_IPSEC, 1)) ||
-#endif
 	    (session->reorder_timeout &&
 	     nla_put_msecs(skb, L2TP_ATTR_RECV_TIMEOUT,
 			   session->reorder_timeout, L2TP_ATTR_PAD)))
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 6e2c8e7595e0..c33ef9a3f3b5 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -95,7 +95,6 @@
 #include <net/netns/generic.h>
 #include <net/ip.h>
 #include <net/udp.h>
-#include <net/xfrm.h>
 #include <net/inet_common.h>
 
 #include <asm/byteorder.h>
@@ -1153,9 +1152,7 @@ static int pppol2tp_tunnel_ioctl(struct l2tp_tunnel *tunnel,
 			l2tp_session_dec_refcount(session);
 			break;
 		}
-#ifdef CONFIG_XFRM
-		stats.using_ipsec = (sk->sk_policy[0] || sk->sk_policy[1]) ? 1 : 0;
-#endif
+		stats.using_ipsec = l2tp_tunnel_uses_xfrm(tunnel);
 		pppol2tp_copy_stats(&stats, &tunnel->stats);
 		if (copy_to_user((void __user *) arg, &stats, sizeof(stats))) {
 			err = -EFAULT;
-- 
2.18.0

^ permalink raw reply related

* [PATCH net-next 0/8] l2tp: rework pppol2tp ioctl handling
From: Guillaume Nault @ 2018-08-10 11:21 UTC (permalink / raw)
  To: netdev; +Cc: James Chapman

The current ioctl() handling code can be simplified. It tests for
non-relevant conditions and uselessly holds sockets. Once useless
code is removed, it becomes even simpler to let pppol2tp_ioctl() handle
commands directly, rather than dispatch them to pppol2tp_tunnel_ioctl()
or pppol2tp_session_ioctl(). That is the approach taken by this series.

Patch #1 and #2 define helper functions aimed at simplifying the rest
of the patch set.

Patch #3 drops useless tests in pppol2p_ioctl() and avoid holding a
refcount on the socket.

Patches #4, #5 and #6 are the core of the series. They let
pppol2tp_ioctl() handle all ioctls and drop the tunnel and session
specific functions.

Then patch #6 brings a little bit of consolidation.

Finally, patch #7 takes advantage of the simplified code to make
pppol2tp sockets compatible with dev_ioctl(). Certainly not a killer
feature, but it is trivial and it is always nice to see l2tp getting
better integration with the rest of the stack.

Guillaume Nault (8):
  l2tp: define l2tp_tunnel_uses_xfrm()
  l2tp: split l2tp_session_get()
  l2tp: simplify pppol2tp_ioctl()
  l2tp: handle PPPIOC[GS]MRU and PPPIOC[GS]FLAGS in pppol2tp_ioctl()
  l2tp: remove pppol2tp_tunnel_ioctl()
  l2tp: remove pppol2tp_session_ioctl()
  l2tp: zero out stats in pppol2tp_copy_stats()
  l2tp: let pppol2tp_ioctl() fallback to dev_ioctl()

 include/uapi/linux/ppp-ioctl.h |   2 +-
 net/l2tp/l2tp_core.c           |  50 +++----
 net/l2tp/l2tp_core.h           |  25 +++-
 net/l2tp/l2tp_ip.c             |   2 +-
 net/l2tp/l2tp_ip6.c            |   2 +-
 net/l2tp/l2tp_netlink.c        |  11 +-
 net/l2tp/l2tp_ppp.c            | 240 +++++++++++----------------------
 7 files changed, 133 insertions(+), 199 deletions(-)

-- 
2.18.0

^ permalink raw reply

* [PATCH 8/8] netfilter: nfnetlink_osf: fix using plain integer as NULL warning
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>

From: Wei Yongjun <weiyongjun1@huawei.com>

Fixes the following sparse warning:

net/netfilter/nfnetlink_osf.c:274:24: warning:
 Using plain integer as NULL pointer

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_osf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
index f9dba62c450f..00db27dfd2ff 100644
--- a/net/netfilter/nfnetlink_osf.c
+++ b/net/netfilter/nfnetlink_osf.c
@@ -271,7 +271,7 @@ const char *nf_osf_find(const struct sk_buff *skb,
 
 	tcp = nf_osf_hdr_ctx_init(&ctx, skb, ip, opts);
 	if (!tcp)
-		return false;
+		return NULL;
 
 	list_for_each_entry_rcu(kf, &nf_osf_fingers[ctx.df], finger_entry) {
 		f = &kf->finger;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 5/8] netfilter: nft_ct: add ct timeout support
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>

From: Harsha Sharma <harshasharmaiitr@gmail.com>

This patch allows to add, list and delete connection tracking timeout
policies via nft objref infrastructure and assigning these timeout
via nft rule.

%./libnftnl/examples/nft-ct-timeout-add ip raw cttime tcp

Ruleset:

table ip raw {
   ct timeout cttime {
       protocol tcp;
       policy = {established: 111, close: 13 }
   }

   chain output {
       type filter hook output priority -300; policy accept;
       ct timeout set "cttime"
   }
}

%./libnftnl/examples/nft-rule-ct-timeout-add ip raw output cttime

%conntrack -E
[NEW] tcp      6 111 ESTABLISHED src=172.16.19.128 dst=172.16.19.1
sport=22 dport=41360 [UNREPLIED] src=172.16.19.1 dst=172.16.19.128
sport=41360 dport=22

%nft delete rule ip raw output handle <handle>
%./libnftnl/examples/nft-ct-timeout-del ip raw cttime

Joint work with Pablo Neira.

Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter/nf_tables.h |  14 ++-
 net/netfilter/nft_ct.c                   | 204 ++++++++++++++++++++++++++++++-
 2 files changed, 216 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 94657c701f22..e23290ffdc77 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -958,6 +958,7 @@ enum nft_socket_keys {
  * @NFT_CT_DST_IP: conntrack layer 3 protocol destination (IPv4 address)
  * @NFT_CT_SRC_IP6: conntrack layer 3 protocol source (IPv6 address)
  * @NFT_CT_DST_IP6: conntrack layer 3 protocol destination (IPv6 address)
+ * @NFT_CT_TIMEOUT: connection tracking timeout policy assigned to conntrack
  */
 enum nft_ct_keys {
 	NFT_CT_STATE,
@@ -983,6 +984,7 @@ enum nft_ct_keys {
 	NFT_CT_DST_IP,
 	NFT_CT_SRC_IP6,
 	NFT_CT_DST_IP6,
+	NFT_CT_TIMEOUT,
 	__NFT_CT_MAX
 };
 #define NFT_CT_MAX		(__NFT_CT_MAX - 1)
@@ -1411,6 +1413,15 @@ enum nft_ct_helper_attributes {
 };
 #define NFTA_CT_HELPER_MAX	(__NFTA_CT_HELPER_MAX - 1)
 
+enum nft_ct_timeout_timeout_attributes {
+	NFTA_CT_TIMEOUT_UNSPEC,
+	NFTA_CT_TIMEOUT_L3PROTO,
+	NFTA_CT_TIMEOUT_L4PROTO,
+	NFTA_CT_TIMEOUT_DATA,
+	__NFTA_CT_TIMEOUT_MAX,
+};
+#define NFTA_CT_TIMEOUT_MAX	(__NFTA_CT_TIMEOUT_MAX - 1)
+
 #define NFT_OBJECT_UNSPEC	0
 #define NFT_OBJECT_COUNTER	1
 #define NFT_OBJECT_QUOTA	2
@@ -1418,7 +1429,8 @@ enum nft_ct_helper_attributes {
 #define NFT_OBJECT_LIMIT	4
 #define NFT_OBJECT_CONNLIMIT	5
 #define NFT_OBJECT_TUNNEL	6
-#define __NFT_OBJECT_MAX	7
+#define NFT_OBJECT_CT_TIMEOUT	7
+#define __NFT_OBJECT_MAX	8
 #define NFT_OBJECT_MAX		(__NFT_OBJECT_MAX - 1)
 
 /**
diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 3bc82ee5464d..4788458a0931 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -22,6 +22,8 @@
 #include <net/netfilter/nf_conntrack_helper.h>
 #include <net/netfilter/nf_conntrack_ecache.h>
 #include <net/netfilter/nf_conntrack_labels.h>
+#include <net/netfilter/nf_conntrack_timeout.h>
+#include <net/netfilter/nf_conntrack_l4proto.h>
 
 struct nft_ct {
 	enum nft_ct_keys	key:8;
@@ -765,6 +767,194 @@ static struct nft_expr_type nft_notrack_type __read_mostly = {
 	.owner		= THIS_MODULE,
 };
 
+#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
+static int
+nft_ct_timeout_parse_policy(void *timeouts,
+			    const struct nf_conntrack_l4proto *l4proto,
+			    struct net *net, const struct nlattr *attr)
+{
+	struct nlattr **tb;
+	int ret = 0;
+
+	if (!l4proto->ctnl_timeout.nlattr_to_obj)
+		return 0;
+
+	tb = kcalloc(l4proto->ctnl_timeout.nlattr_max + 1, sizeof(*tb),
+		     GFP_KERNEL);
+
+	if (!tb)
+		return -ENOMEM;
+
+	ret = nla_parse_nested(tb, l4proto->ctnl_timeout.nlattr_max,
+			       attr, l4proto->ctnl_timeout.nla_policy,
+			       NULL);
+	if (ret < 0)
+		goto err;
+
+	ret = l4proto->ctnl_timeout.nlattr_to_obj(tb, net, timeouts);
+
+err:
+	kfree(tb);
+	return ret;
+}
+
+struct nft_ct_timeout_obj {
+	struct nf_conn		*tmpl;
+	u8			l4proto;
+};
+
+static void nft_ct_timeout_obj_eval(struct nft_object *obj,
+				    struct nft_regs *regs,
+				    const struct nft_pktinfo *pkt)
+{
+	const struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
+	struct nf_conn *ct = (struct nf_conn *)skb_nfct(pkt->skb);
+	struct sk_buff *skb = pkt->skb;
+
+	if (ct ||
+	    priv->l4proto != pkt->tprot)
+		return;
+
+	nf_ct_set(skb, priv->tmpl, IP_CT_NEW);
+}
+
+static int nft_ct_timeout_obj_init(const struct nft_ctx *ctx,
+				   const struct nlattr * const tb[],
+				   struct nft_object *obj)
+{
+	const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;
+	struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
+	const struct nf_conntrack_l4proto *l4proto;
+	struct nf_conn_timeout *timeout_ext;
+	struct nf_ct_timeout *timeout;
+	int l3num = ctx->family;
+	struct nf_conn *tmpl;
+	__u8 l4num;
+	int ret;
+
+	if (!tb[NFTA_CT_TIMEOUT_L3PROTO] ||
+	    !tb[NFTA_CT_TIMEOUT_L4PROTO] ||
+	    !tb[NFTA_CT_TIMEOUT_DATA])
+		return -EINVAL;
+
+	l3num = ntohs(nla_get_be16(tb[NFTA_CT_TIMEOUT_L3PROTO]));
+	l4num = nla_get_u8(tb[NFTA_CT_TIMEOUT_L4PROTO]);
+	priv->l4proto = l4num;
+
+	l4proto = nf_ct_l4proto_find_get(l3num, l4num);
+
+	if (l4proto->l4proto != l4num) {
+		ret = -EOPNOTSUPP;
+		goto err_proto_put;
+	}
+
+	timeout = kzalloc(sizeof(struct nf_ct_timeout) +
+			  l4proto->ctnl_timeout.obj_size, GFP_KERNEL);
+	if (timeout == NULL) {
+		ret = -ENOMEM;
+		goto err_proto_put;
+	}
+
+	ret = nft_ct_timeout_parse_policy(&timeout->data, l4proto, ctx->net,
+					  tb[NFTA_CT_TIMEOUT_DATA]);
+	if (ret < 0)
+		goto err_free_timeout;
+
+	timeout->l3num = l3num;
+	timeout->l4proto = l4proto;
+	tmpl = nf_ct_tmpl_alloc(ctx->net, zone, GFP_ATOMIC);
+	if (!tmpl) {
+		ret = -ENOMEM;
+		goto err_free_timeout;
+	}
+
+	timeout_ext = nf_ct_timeout_ext_add(tmpl, timeout, GFP_ATOMIC);
+	if (!timeout_ext) {
+		ret = -ENOMEM;
+		goto err_free_tmpl;
+	}
+
+	ret = nf_ct_netns_get(ctx->net, ctx->family);
+	if (ret < 0)
+		goto err_free_tmpl;
+
+	priv->tmpl = tmpl;
+
+	return 0;
+
+err_free_tmpl:
+	nf_ct_tmpl_free(tmpl);
+err_free_timeout:
+	kfree(timeout);
+err_proto_put:
+	nf_ct_l4proto_put(l4proto);
+	return ret;
+}
+
+static void nft_ct_timeout_obj_destroy(const struct nft_ctx *ctx,
+				       struct nft_object *obj)
+{
+	struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
+	struct nf_conn_timeout *t = nf_ct_timeout_find(priv->tmpl);
+	struct nf_ct_timeout *timeout;
+
+	timeout = rcu_dereference_raw(t->timeout);
+	nf_ct_untimeout(ctx->net, timeout);
+	nf_ct_l4proto_put(timeout->l4proto);
+	nf_ct_netns_put(ctx->net, ctx->family);
+	nf_ct_tmpl_free(priv->tmpl);
+}
+
+static int nft_ct_timeout_obj_dump(struct sk_buff *skb,
+				   struct nft_object *obj, bool reset)
+{
+	const struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
+	const struct nf_conn_timeout *t = nf_ct_timeout_find(priv->tmpl);
+	const struct nf_ct_timeout *timeout = rcu_dereference_raw(t->timeout);
+	struct nlattr *nest_params;
+	int ret;
+
+	if (nla_put_u8(skb, NFTA_CT_TIMEOUT_L4PROTO, timeout->l4proto->l4proto) ||
+	    nla_put_be16(skb, NFTA_CT_TIMEOUT_L3PROTO, htons(timeout->l3num)))
+		return -1;
+
+	nest_params = nla_nest_start(skb, NFTA_CT_TIMEOUT_DATA | NLA_F_NESTED);
+	if (!nest_params)
+		return -1;
+
+	ret = timeout->l4proto->ctnl_timeout.obj_to_nlattr(skb, &timeout->data);
+	if (ret < 0)
+		return -1;
+	nla_nest_end(skb, nest_params);
+	return 0;
+}
+
+static const struct nla_policy nft_ct_timeout_policy[NFTA_CT_TIMEOUT_MAX + 1] = {
+	[NFTA_CT_TIMEOUT_L3PROTO] = {.type = NLA_U16 },
+	[NFTA_CT_TIMEOUT_L4PROTO] = {.type = NLA_U8 },
+	[NFTA_CT_TIMEOUT_DATA]	  = {.type = NLA_NESTED },
+};
+
+static struct nft_object_type nft_ct_timeout_obj_type;
+
+static const struct nft_object_ops nft_ct_timeout_obj_ops = {
+	.type		= &nft_ct_timeout_obj_type,
+	.size		= sizeof(struct nft_ct_timeout_obj),
+	.eval		= nft_ct_timeout_obj_eval,
+	.init		= nft_ct_timeout_obj_init,
+	.destroy	= nft_ct_timeout_obj_destroy,
+	.dump		= nft_ct_timeout_obj_dump,
+};
+
+static struct nft_object_type nft_ct_timeout_obj_type __read_mostly = {
+	.type		= NFT_OBJECT_CT_TIMEOUT,
+	.ops		= &nft_ct_timeout_obj_ops,
+	.maxattr	= NFTA_CT_TIMEOUT_MAX,
+	.policy		= nft_ct_timeout_policy,
+	.owner		= THIS_MODULE,
+};
+#endif /* CONFIG_NF_CONNTRACK_TIMEOUT */
+
 static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
 				  const struct nlattr * const tb[],
 				  struct nft_object *obj)
@@ -949,9 +1139,17 @@ static int __init nft_ct_module_init(void)
 	err = nft_register_obj(&nft_ct_helper_obj_type);
 	if (err < 0)
 		goto err2;
-
+#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
+	err = nft_register_obj(&nft_ct_timeout_obj_type);
+	if (err < 0)
+		goto err3;
+#endif
 	return 0;
 
+#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
+err3:
+	nft_unregister_obj(&nft_ct_helper_obj_type);
+#endif
 err2:
 	nft_unregister_expr(&nft_notrack_type);
 err1:
@@ -961,6 +1159,9 @@ static int __init nft_ct_module_init(void)
 
 static void __exit nft_ct_module_exit(void)
 {
+#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
+	nft_unregister_obj(&nft_ct_timeout_obj_type);
+#endif
 	nft_unregister_obj(&nft_ct_helper_obj_type);
 	nft_unregister_expr(&nft_notrack_type);
 	nft_unregister_expr(&nft_ct_type);
@@ -974,3 +1175,4 @@ MODULE_AUTHOR("Patrick McHardy <kaber@trash.net>");
 MODULE_ALIAS_NFT_EXPR("ct");
 MODULE_ALIAS_NFT_EXPR("notrack");
 MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_HELPER);
+MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_TIMEOUT);
-- 
2.11.0

^ permalink raw reply related

* [PATCH 3/8] netfilter: cttimeout: decouple timeout policy from nfnetlink_cttimeout object
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>

The timeout policy is currently embedded into the nfnetlink_cttimeout
object, move the policy into an independent object. This allows us to
reuse part of the existing conntrack timeout extension from nf_tables
without adding dependencies with the nfnetlink_cttimeout object layout.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_timeout.h | 22 ++++++++++-------
 net/netfilter/nf_conntrack_timeout.c         |  6 ++---
 net/netfilter/nfnetlink_cttimeout.c          | 37 ++++++++++++++++------------
 net/netfilter/xt_CT.c                        |  4 +--
 4 files changed, 39 insertions(+), 30 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_timeout.h b/include/net/netfilter/nf_conntrack_timeout.h
index 7a21bc0f00eb..d5f62cc6c2ae 100644
--- a/include/net/netfilter/nf_conntrack_timeout.h
+++ b/include/net/netfilter/nf_conntrack_timeout.h
@@ -11,24 +11,28 @@
 
 #define CTNL_TIMEOUT_NAME_MAX	32
 
+struct nf_ct_timeout {
+	__u16			l3num;
+	const struct nf_conntrack_l4proto *l4proto;
+	char			data[0];
+};
+
 struct ctnl_timeout {
 	struct list_head	head;
 	struct rcu_head		rcu_head;
 	refcount_t		refcnt;
 	char			name[CTNL_TIMEOUT_NAME_MAX];
-	__u16			l3num;
-	const struct nf_conntrack_l4proto *l4proto;
-	char			data[0];
+	struct nf_ct_timeout	timeout;
 };
 
 struct nf_conn_timeout {
-	struct ctnl_timeout __rcu *timeout;
+	struct nf_ct_timeout __rcu *timeout;
 };
 
 static inline unsigned int *
 nf_ct_timeout_data(struct nf_conn_timeout *t)
 {
-	struct ctnl_timeout *timeout;
+	struct nf_ct_timeout *timeout;
 
 	timeout = rcu_dereference(t->timeout);
 	if (timeout == NULL)
@@ -49,7 +53,7 @@ struct nf_conn_timeout *nf_ct_timeout_find(const struct nf_conn *ct)
 
 static inline
 struct nf_conn_timeout *nf_ct_timeout_ext_add(struct nf_conn *ct,
-					      struct ctnl_timeout *timeout,
+					      struct nf_ct_timeout *timeout,
 					      gfp_t gfp)
 {
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
@@ -83,7 +87,7 @@ static inline unsigned int *nf_ct_timeout_lookup(const struct nf_conn *ct)
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
 int nf_conntrack_timeout_init(void);
 void nf_conntrack_timeout_fini(void);
-void nf_ct_untimeout(struct net *net, struct ctnl_timeout *timeout);
+void nf_ct_untimeout(struct net *net, struct nf_ct_timeout *timeout);
 #else
 static inline int nf_conntrack_timeout_init(void)
 {
@@ -97,8 +101,8 @@ static inline void nf_conntrack_timeout_fini(void)
 #endif /* CONFIG_NF_CONNTRACK_TIMEOUT */
 
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
-extern struct ctnl_timeout *(*nf_ct_timeout_find_get_hook)(struct net *net, const char *name);
-extern void (*nf_ct_timeout_put_hook)(struct ctnl_timeout *timeout);
+extern struct nf_ct_timeout *(*nf_ct_timeout_find_get_hook)(struct net *net, const char *name);
+extern void (*nf_ct_timeout_put_hook)(struct nf_ct_timeout *timeout);
 #endif
 
 #endif /* _NF_CONNTRACK_TIMEOUT_H */
diff --git a/net/netfilter/nf_conntrack_timeout.c b/net/netfilter/nf_conntrack_timeout.c
index 401c2cce4a61..91fbd183da2d 100644
--- a/net/netfilter/nf_conntrack_timeout.c
+++ b/net/netfilter/nf_conntrack_timeout.c
@@ -24,11 +24,11 @@
 #include <net/netfilter/nf_conntrack_extend.h>
 #include <net/netfilter/nf_conntrack_timeout.h>
 
-struct ctnl_timeout *
+struct nf_ct_timeout *
 (*nf_ct_timeout_find_get_hook)(struct net *net, const char *name) __read_mostly;
 EXPORT_SYMBOL_GPL(nf_ct_timeout_find_get_hook);
 
-void (*nf_ct_timeout_put_hook)(struct ctnl_timeout *timeout) __read_mostly;
+void (*nf_ct_timeout_put_hook)(struct nf_ct_timeout *timeout) __read_mostly;
 EXPORT_SYMBOL_GPL(nf_ct_timeout_put_hook);
 
 static int untimeout(struct nf_conn *ct, void *timeout)
@@ -42,7 +42,7 @@ static int untimeout(struct nf_conn *ct, void *timeout)
 	return 0;
 }
 
-void nf_ct_untimeout(struct net *net, struct ctnl_timeout *timeout)
+void nf_ct_untimeout(struct net *net, struct nf_ct_timeout *timeout)
 {
 	nf_ct_iterate_cleanup_net(net, untimeout, timeout, 0, 0);
 }
diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index df53aef2d642..d46a236cdf31 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -113,13 +113,13 @@ static int cttimeout_new_timeout(struct net *net, struct sock *ctnl,
 			/* You cannot replace one timeout policy by another of
 			 * different kind, sorry.
 			 */
-			if (matching->l3num != l3num ||
-			    matching->l4proto->l4proto != l4num)
+			if (matching->timeout.l3num != l3num ||
+			    matching->timeout.l4proto->l4proto != l4num)
 				return -EINVAL;
 
-			return ctnl_timeout_parse_policy(&matching->data,
-							 matching->l4proto, net,
-							 cda[CTA_TIMEOUT_DATA]);
+			return ctnl_timeout_parse_policy(&matching->timeout.data,
+							 matching->timeout.l4proto,
+							 net, cda[CTA_TIMEOUT_DATA]);
 		}
 
 		return -EBUSY;
@@ -140,14 +140,14 @@ static int cttimeout_new_timeout(struct net *net, struct sock *ctnl,
 		goto err_proto_put;
 	}
 
-	ret = ctnl_timeout_parse_policy(&timeout->data, l4proto, net,
+	ret = ctnl_timeout_parse_policy(&timeout->timeout.data, l4proto, net,
 					cda[CTA_TIMEOUT_DATA]);
 	if (ret < 0)
 		goto err;
 
 	strcpy(timeout->name, nla_data(cda[CTA_TIMEOUT_NAME]));
-	timeout->l3num = l3num;
-	timeout->l4proto = l4proto;
+	timeout->timeout.l3num = l3num;
+	timeout->timeout.l4proto = l4proto;
 	refcount_set(&timeout->refcnt, 1);
 	list_add_tail_rcu(&timeout->head, &net->nfct_timeout_list);
 
@@ -166,7 +166,7 @@ ctnl_timeout_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
 	struct nlmsghdr *nlh;
 	struct nfgenmsg *nfmsg;
 	unsigned int flags = portid ? NLM_F_MULTI : 0;
-	const struct nf_conntrack_l4proto *l4proto = timeout->l4proto;
+	const struct nf_conntrack_l4proto *l4proto = timeout->timeout.l4proto;
 
 	event = nfnl_msg_type(NFNL_SUBSYS_CTNETLINK_TIMEOUT, event);
 	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*nfmsg), flags);
@@ -179,8 +179,9 @@ ctnl_timeout_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
 	nfmsg->res_id = 0;
 
 	if (nla_put_string(skb, CTA_TIMEOUT_NAME, timeout->name) ||
-	    nla_put_be16(skb, CTA_TIMEOUT_L3PROTO, htons(timeout->l3num)) ||
-	    nla_put_u8(skb, CTA_TIMEOUT_L4PROTO, timeout->l4proto->l4proto) ||
+	    nla_put_be16(skb, CTA_TIMEOUT_L3PROTO,
+			 htons(timeout->timeout.l3num)) ||
+	    nla_put_u8(skb, CTA_TIMEOUT_L4PROTO, l4proto->l4proto) ||
 	    nla_put_be32(skb, CTA_TIMEOUT_USE,
 			 htonl(refcount_read(&timeout->refcnt))))
 		goto nla_put_failure;
@@ -194,7 +195,8 @@ ctnl_timeout_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
 		if (!nest_parms)
 			goto nla_put_failure;
 
-		ret = l4proto->ctnl_timeout.obj_to_nlattr(skb, &timeout->data);
+		ret = l4proto->ctnl_timeout.obj_to_nlattr(skb,
+							&timeout->timeout.data);
 		if (ret < 0)
 			goto nla_put_failure;
 
@@ -308,8 +310,8 @@ static int ctnl_timeout_try_del(struct net *net, struct ctnl_timeout *timeout)
 	if (refcount_dec_if_one(&timeout->refcnt)) {
 		/* We are protected by nfnl mutex. */
 		list_del_rcu(&timeout->head);
-		nf_ct_l4proto_put(timeout->l4proto);
-		nf_ct_untimeout(net, timeout);
+		nf_ct_l4proto_put(timeout->timeout.l4proto);
+		nf_ct_untimeout(net, &timeout->timeout);
 		kfree_rcu(timeout, rcu_head);
 	} else {
 		ret = -EBUSY;
@@ -510,8 +512,11 @@ ctnl_timeout_find_get(struct net *net, const char *name)
 	return matching;
 }
 
-static void ctnl_timeout_put(struct ctnl_timeout *timeout)
+static void ctnl_timeout_put(struct nf_ct_timeout *t)
 {
+	struct ctnl_timeout *timeout =
+		container_of(t, struct ctnl_timeout, timeout);
+
 	if (refcount_dec_and_test(&timeout->refcnt))
 		kfree_rcu(timeout, rcu_head);
 
@@ -561,7 +566,7 @@ static void __net_exit cttimeout_net_exit(struct net *net)
 
 	list_for_each_entry_safe(cur, tmp, &net->nfct_timeout_list, head) {
 		list_del_rcu(&cur->head);
-		nf_ct_l4proto_put(cur->l4proto);
+		nf_ct_l4proto_put(cur->timeout.l4proto);
 
 		if (refcount_dec_and_test(&cur->refcnt))
 			kfree_rcu(cur, rcu_head);
diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index 7ba454e9e3fa..89457efd2e00 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -104,7 +104,7 @@ xt_ct_set_helper(struct nf_conn *ct, const char *helper_name,
 }
 
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
-static void __xt_ct_tg_timeout_put(struct ctnl_timeout *timeout)
+static void __xt_ct_tg_timeout_put(struct nf_ct_timeout *timeout)
 {
 	typeof(nf_ct_timeout_put_hook) timeout_put;
 
@@ -121,7 +121,7 @@ xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
 	typeof(nf_ct_timeout_find_get_hook) timeout_find_get;
 	const struct nf_conntrack_l4proto *l4proto;
-	struct ctnl_timeout *timeout;
+	struct nf_ct_timeout *timeout;
 	struct nf_conn_timeout *timeout_ext;
 	const char *errmsg = NULL;
 	int ret = 0;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 7/8] netfilter: nfnetlink_osf: add missing enum in nfnetlink_osf uapi header
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>

From: Fernando Fernandez Mancera <ffmancera@riseup.net>

xt_osf_window_size_options was originally part of
include/uapi/linux/netfilter/xt_osf.h, restore it.

Fixes: bfb15f2a95cb ("netfilter: extract Passive OS fingerprint infrastructure from xt_osf")
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/nfnetlink_osf.h      | 12 ------------
 include/uapi/linux/netfilter/nfnetlink_osf.h | 12 ++++++++++++
 include/uapi/linux/netfilter/xt_osf.h        |  1 +
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_osf.h b/include/linux/netfilter/nfnetlink_osf.h
index a7311bc03d3a..ecf7dab81e9e 100644
--- a/include/linux/netfilter/nfnetlink_osf.h
+++ b/include/linux/netfilter/nfnetlink_osf.h
@@ -4,18 +4,6 @@
 
 #include <uapi/linux/netfilter/nfnetlink_osf.h>
 
-/* Initial window size option state machine: multiple of mss, mtu or
- * plain numeric value. Can also be made as plain numeric value which
- * is not a multiple of specified value.
- */
-enum nf_osf_window_size_options {
-	OSF_WSS_PLAIN   = 0,
-	OSF_WSS_MSS,
-	OSF_WSS_MTU,
-	OSF_WSS_MODULO,
-	OSF_WSS_MAX,
-};
-
 enum osf_fmatch_states {
 	/* Packet does not match the fingerprint */
 	FMATCH_WRONG = 0,
diff --git a/include/uapi/linux/netfilter/nfnetlink_osf.h b/include/uapi/linux/netfilter/nfnetlink_osf.h
index 3b93fbb9fc24..76a3527df5dd 100644
--- a/include/uapi/linux/netfilter/nfnetlink_osf.h
+++ b/include/uapi/linux/netfilter/nfnetlink_osf.h
@@ -88,6 +88,18 @@ enum iana_options {
 	OSFOPT_EMPTY = 255,
 };
 
+/* Initial window size option state machine: multiple of mss, mtu or
+ * plain numeric value. Can also be made as plain numeric value which
+ * is not a multiple of specified value.
+ */
+enum nf_osf_window_size_options {
+	OSF_WSS_PLAIN	= 0,
+	OSF_WSS_MSS,
+	OSF_WSS_MTU,
+	OSF_WSS_MODULO,
+	OSF_WSS_MAX,
+};
+
 enum nf_osf_attr_type {
 	OSF_ATTR_UNSPEC,
 	OSF_ATTR_FINGER,
diff --git a/include/uapi/linux/netfilter/xt_osf.h b/include/uapi/linux/netfilter/xt_osf.h
index c56c59605c2b..24102b5286ec 100644
--- a/include/uapi/linux/netfilter/xt_osf.h
+++ b/include/uapi/linux/netfilter/xt_osf.h
@@ -46,6 +46,7 @@
 #define xt_osf_finger		nf_osf_finger
 #define xt_osf_nlmsg		nf_osf_nlmsg
 
+#define xt_osf_window_size_options	nf_osf_window_size_options
 #define xt_osf_attr_type	nf_osf_attr_type
 #define xt_osf_msg_types	nf_osf_msg_types
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 6/8] netfilter: nft_ct: enable conntrack for helpers
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>

Enable conntrack if the user defines a helper to be used from the
ruleset policy.

Fixes: 1a64edf54f55 ("netfilter: nft_ct: add helper set support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_ct.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
index 4788458a0931..4855d4ce1c8f 100644
--- a/net/netfilter/nft_ct.c
+++ b/net/netfilter/nft_ct.c
@@ -963,6 +963,7 @@ static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
 	struct nf_conntrack_helper *help4, *help6;
 	char name[NF_CT_HELPER_NAME_LEN];
 	int family = ctx->family;
+	int err;
 
 	if (!tb[NFTA_CT_HELPER_NAME] || !tb[NFTA_CT_HELPER_L4PROTO])
 		return -EINVAL;
@@ -1013,7 +1014,18 @@ static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
 	priv->helper4 = help4;
 	priv->helper6 = help6;
 
+	err = nf_ct_netns_get(ctx->net, ctx->family);
+	if (err < 0)
+		goto err_put_helper;
+
 	return 0;
+
+err_put_helper:
+	if (priv->helper4)
+		nf_conntrack_helper_put(priv->helper4);
+	if (priv->helper6)
+		nf_conntrack_helper_put(priv->helper6);
+	return err;
 }
 
 static void nft_ct_helper_obj_destroy(const struct nft_ctx *ctx,
@@ -1025,6 +1037,8 @@ static void nft_ct_helper_obj_destroy(const struct nft_ctx *ctx,
 		nf_conntrack_helper_put(priv->helper4);
 	if (priv->helper6)
 		nf_conntrack_helper_put(priv->helper6);
+
+	nf_ct_netns_put(ctx->net, ctx->family);
 }
 
 static void nft_ct_helper_obj_eval(struct nft_object *obj,
-- 
2.11.0

^ permalink raw reply related

* [PATCH 4/8] netfilter: remove ifdef around cttimeout in struct nf_conntrack_l4proto
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>

Simplify this, include it inconditionally in this structure layout as we
do with ctnetlink.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_l4proto.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_l4proto.h b/include/net/netfilter/nf_conntrack_l4proto.h
index 6068c6da3eac..8465263b297d 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -77,7 +77,6 @@ struct nf_conntrack_l4proto {
 			       struct nf_conntrack_tuple *t);
 	const struct nla_policy *nla_policy;
 
-#if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)
 	struct {
 		int (*nlattr_to_obj)(struct nlattr *tb[],
 				     struct net *net, void *data);
@@ -87,7 +86,6 @@ struct nf_conntrack_l4proto {
 		u16 nlattr_max;
 		const struct nla_policy *nla_policy;
 	} ctnl_timeout;
-#endif
 #ifdef CONFIG_NF_CONNTRACK_PROCFS
 	/* Print out the private part of the conntrack. */
 	void (*print_conntrack)(struct seq_file *s, struct nf_conn *);
-- 
2.11.0

^ permalink raw reply related

* [PATCH 2/8] netfilter: cttimeout: move ctnl_untimeout to nf_conntrack
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>

From: Harsha Sharma <harshasharmaiitr@gmail.com>

As, ctnl_untimeout is required by nft_ct, so move ctnl_timeout from
nfnetlink_cttimeout to nf_conntrack_timeout and rename as nf_ct_timeout.

Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_timeout.h |  1 +
 net/netfilter/nf_conntrack_timeout.c         | 17 +++++++++++++++++
 net/netfilter/nfnetlink_cttimeout.c          | 20 ++------------------
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_timeout.h b/include/net/netfilter/nf_conntrack_timeout.h
index 80ceb3d0291d..7a21bc0f00eb 100644
--- a/include/net/netfilter/nf_conntrack_timeout.h
+++ b/include/net/netfilter/nf_conntrack_timeout.h
@@ -83,6 +83,7 @@ static inline unsigned int *nf_ct_timeout_lookup(const struct nf_conn *ct)
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
 int nf_conntrack_timeout_init(void);
 void nf_conntrack_timeout_fini(void);
+void nf_ct_untimeout(struct net *net, struct ctnl_timeout *timeout);
 #else
 static inline int nf_conntrack_timeout_init(void)
 {
diff --git a/net/netfilter/nf_conntrack_timeout.c b/net/netfilter/nf_conntrack_timeout.c
index 46aee65f339b..401c2cce4a61 100644
--- a/net/netfilter/nf_conntrack_timeout.c
+++ b/net/netfilter/nf_conntrack_timeout.c
@@ -31,6 +31,23 @@ EXPORT_SYMBOL_GPL(nf_ct_timeout_find_get_hook);
 void (*nf_ct_timeout_put_hook)(struct ctnl_timeout *timeout) __read_mostly;
 EXPORT_SYMBOL_GPL(nf_ct_timeout_put_hook);
 
+static int untimeout(struct nf_conn *ct, void *timeout)
+{
+	struct nf_conn_timeout *timeout_ext = nf_ct_timeout_find(ct);
+
+	if (timeout_ext && (!timeout || timeout_ext->timeout == timeout))
+		RCU_INIT_POINTER(timeout_ext->timeout, NULL);
+
+	/* We are not intended to delete this conntrack. */
+	return 0;
+}
+
+void nf_ct_untimeout(struct net *net, struct ctnl_timeout *timeout)
+{
+	nf_ct_iterate_cleanup_net(net, untimeout, timeout, 0, 0);
+}
+EXPORT_SYMBOL_GPL(nf_ct_untimeout);
+
 static const struct nf_ct_ext_type timeout_extend = {
 	.len	= sizeof(struct nf_conn_timeout),
 	.align	= __alignof__(struct nf_conn_timeout),
diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index 4199e5300575..df53aef2d642 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -297,22 +297,6 @@ static int cttimeout_get_timeout(struct net *net, struct sock *ctnl,
 	return ret;
 }
 
-static int untimeout(struct nf_conn *ct, void *timeout)
-{
-	struct nf_conn_timeout *timeout_ext = nf_ct_timeout_find(ct);
-
-	if (timeout_ext && (!timeout || timeout_ext->timeout == timeout))
-		RCU_INIT_POINTER(timeout_ext->timeout, NULL);
-
-	/* We are not intended to delete this conntrack. */
-	return 0;
-}
-
-static void ctnl_untimeout(struct net *net, struct ctnl_timeout *timeout)
-{
-	nf_ct_iterate_cleanup_net(net, untimeout, timeout, 0, 0);
-}
-
 /* try to delete object, fail if it is still in use. */
 static int ctnl_timeout_try_del(struct net *net, struct ctnl_timeout *timeout)
 {
@@ -325,7 +309,7 @@ static int ctnl_timeout_try_del(struct net *net, struct ctnl_timeout *timeout)
 		/* We are protected by nfnl mutex. */
 		list_del_rcu(&timeout->head);
 		nf_ct_l4proto_put(timeout->l4proto);
-		ctnl_untimeout(net, timeout);
+		nf_ct_untimeout(net, timeout);
 		kfree_rcu(timeout, rcu_head);
 	} else {
 		ret = -EBUSY;
@@ -573,7 +557,7 @@ static void __net_exit cttimeout_net_exit(struct net *net)
 	struct ctnl_timeout *cur, *tmp;
 
 	nf_ct_unconfirmed_destroy(net);
-	ctnl_untimeout(net, NULL);
+	nf_ct_untimeout(net, NULL);
 
 	list_for_each_entry_safe(cur, tmp, &net->nfct_timeout_list, head) {
 		list_del_rcu(&cur->head);
-- 
2.11.0

^ permalink raw reply related

* [PATCH 1/8] netfilter: nft_osf: use NFT_OSF_MAXGENRELEN instead of IFNAMSIZ
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <20180810111622.3981-1-pablo@netfilter.org>

From: Fernando Fernandez Mancera <ffmancera@riseup.net>

As no "genre" on pf.os exceed 16 bytes of length, we reduce
NFT_OSF_MAXGENRELEN parameter to 16 bytes and use it instead of IFNAMSIZ.

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter/nf_tables.h | 1 +
 net/netfilter/nft_osf.c                  | 8 +++-----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 357862d948de..94657c701f22 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -8,6 +8,7 @@
 #define NFT_SET_MAXNAMELEN	NFT_NAME_MAXLEN
 #define NFT_OBJ_MAXNAMELEN	NFT_NAME_MAXLEN
 #define NFT_USERDATA_MAXLEN	256
+#define NFT_OSF_MAXGENRELEN	16
 
 /**
  * enum nft_registers - nf_tables registers
diff --git a/net/netfilter/nft_osf.c b/net/netfilter/nft_osf.c
index 9b2f3de7be4f..5af74b37f423 100644
--- a/net/netfilter/nft_osf.c
+++ b/net/netfilter/nft_osf.c
@@ -4,8 +4,6 @@
 #include <net/netfilter/nf_tables.h>
 #include <linux/netfilter/nfnetlink_osf.h>
 
-#define OSF_GENRE_SIZE		32
-
 struct nft_osf {
 	enum nft_registers	dreg:8;
 };
@@ -37,9 +35,9 @@ static void nft_osf_eval(const struct nft_expr *expr, struct nft_regs *regs,
 
 	os_name = nf_osf_find(skb, nf_osf_fingers);
 	if (!os_name)
-		strncpy((char *)dest, "unknown", IFNAMSIZ);
+		strncpy((char *)dest, "unknown", NFT_OSF_MAXGENRELEN);
 	else
-		strncpy((char *)dest, os_name, IFNAMSIZ);
+		strncpy((char *)dest, os_name, NFT_OSF_MAXGENRELEN);
 }
 
 static int nft_osf_init(const struct nft_ctx *ctx,
@@ -51,7 +49,7 @@ static int nft_osf_init(const struct nft_ctx *ctx,
 
 	priv->dreg = nft_parse_register(tb[NFTA_OSF_DREG]);
 	err = nft_validate_register_store(ctx, priv->dreg, NULL,
-					  NFTA_DATA_VALUE, OSF_GENRE_SIZE);
+					  NFTA_DATA_VALUE, NFT_OSF_MAXGENRELEN);
 	if (err < 0)
 		return err;
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH 0/8] Netfilter updates for net-next
From: Pablo Neira Ayuso @ 2018-08-10 11:16 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following batch contains netfilter updates for your net-next tree:

1) Expose NFT_OSF_MAXGENRELEN maximum OS name length from the new OS
   passive fingerprint matching extension, from Fernando Fernandez.

2) Add extension to support for fine grain conntrack timeout policies
   from nf_tables. As preparation works, this patchset moves
   nf_ct_untimeout() to nf_conntrack_timeout and it also decouples the
   timeout policy from the ctnl_timeout object, most work done by
   Harsha Sharma.

3) Enable connection tracking when conntrack helper is in place.

4) Missing enumeration in uapi header when splitting original xt_osf
   to nfnetlink_osf, also from Fernando.

5) Fix a sparse warning due to incorrect typing in the nf_osf_find(),
   from Wei Yongjun.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git

Thanks.

----------------------------------------------------------------

The following changes since commit 981467033a37d916649647fa3afe1fe99bba1817:

  tc-testing: remove duplicate spaces in skbedit match patterns (2018-08-05 17:39:24 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git HEAD

for you to fetch changes up to e7ea2a52ffaf60a211edc0df97dcf194d1257714:

  netfilter: nfnetlink_osf: fix using plain integer as NULL warning (2018-08-08 19:05:39 +0200)

----------------------------------------------------------------
Fernando Fernandez Mancera (2):
      netfilter: nft_osf: use NFT_OSF_MAXGENRELEN instead of IFNAMSIZ
      netfilter: nfnetlink_osf: add missing enum in nfnetlink_osf uapi header

Harsha Sharma (2):
      netfilter: cttimeout: move ctnl_untimeout to nf_conntrack
      netfilter: nft_ct: add ct timeout support

Pablo Neira Ayuso (3):
      netfilter: cttimeout: decouple timeout policy from nfnetlink_cttimeout object
      netfilter: remove ifdef around cttimeout in struct nf_conntrack_l4proto
      netfilter: nft_ct: enable conntrack for helpers

Wei Yongjun (1):
      netfilter: nfnetlink_osf: fix using plain integer as NULL warning

 include/linux/netfilter/nfnetlink_osf.h      |  12 --
 include/net/netfilter/nf_conntrack_l4proto.h |   2 -
 include/net/netfilter/nf_conntrack_timeout.h |  21 ++-
 include/uapi/linux/netfilter/nf_tables.h     |  15 +-
 include/uapi/linux/netfilter/nfnetlink_osf.h |  12 ++
 include/uapi/linux/netfilter/xt_osf.h        |   1 +
 net/netfilter/nf_conntrack_timeout.c         |  21 ++-
 net/netfilter/nfnetlink_cttimeout.c          |  55 +++----
 net/netfilter/nfnetlink_osf.c                |   2 +-
 net/netfilter/nft_ct.c                       | 218 ++++++++++++++++++++++++++-
 net/netfilter/nft_osf.c                      |   8 +-
 net/netfilter/xt_CT.c                        |   4 +-
 12 files changed, 304 insertions(+), 67 deletions(-)

^ permalink raw reply


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