* [PATCH 1/9] l2tp: don't log data frames
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 12:28 ` kernel test robot
2020-08-21 10:47 ` [PATCH 2/9] l2tp: remove noisy logging, use appropriate log levels Tom Parkin
` (8 subsequent siblings)
9 siblings, 1 reply; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
l2tp had logging to trace data frame receipt and transmission, including
code to dump packet contents. This was originally intended to aid
debugging of core l2tp packet handling, but is of limited use now that
code is stable.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
net/l2tp/l2tp_core.c | 44 +-------------------------------------------
net/l2tp/l2tp_eth.c | 11 -----------
net/l2tp/l2tp_ip.c | 15 ---------------
net/l2tp/l2tp_ip6.c | 15 ---------------
net/l2tp/l2tp_ppp.c | 8 --------
5 files changed, 1 insertion(+), 92 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 701fc72ad9f4..ce647816da61 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -694,10 +694,6 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
/* Store L2TP info in the skb */
L2TP_SKB_CB(skb)->ns = ns;
L2TP_SKB_CB(skb)->has_seq = 1;
-
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: recv data ns=%u, nr=%u, session nr=%u\n",
- session->name, ns, nr, session->nr);
}
} else if (session->l2specific_type == L2TP_L2SPECTYPE_DEFAULT) {
u32 l2h = ntohl(*(__be32 *)ptr);
@@ -708,10 +704,6 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
/* Store L2TP info in the skb */
L2TP_SKB_CB(skb)->ns = ns;
L2TP_SKB_CB(skb)->has_seq = 1;
-
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: recv data ns=%u, session nr=%u\n",
- session->name, ns, session->nr);
}
ptr += 4;
}
@@ -853,16 +845,6 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
goto error;
}
- /* Trace packet contents, if enabled */
- if (tunnel->debug & L2TP_MSG_DATA) {
- length = min(32u, skb->len);
- if (!pskb_may_pull(skb, length))
- goto error;
-
- pr_debug("%s: recv\n", tunnel->name);
- print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, skb->data, length);
- }
-
/* Point to L2TP header */
optr = skb->data;
ptr = skb->data;
@@ -883,12 +865,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
length = skb->len;
/* If type is control packet, it is handled by userspace. */
- if (hdrflags & L2TP_HDRFLAG_T) {
- l2tp_dbg(tunnel, L2TP_MSG_DATA,
- "%s: recv control packet, len=%d\n",
- tunnel->name, length);
+ if (hdrflags & L2TP_HDRFLAG_T)
goto error;
- }
/* Skip flags */
ptr += 2;
@@ -953,9 +931,6 @@ int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
if (!tunnel)
goto pass_up;
- l2tp_dbg(tunnel, L2TP_MSG_DATA, "%s: received %d bytes\n",
- tunnel->name, skb->len);
-
if (l2tp_udp_recv_core(tunnel, skb))
goto pass_up;
@@ -1049,23 +1024,6 @@ static void l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb,
unsigned int len = skb->len;
int error;
- /* Debug */
- if (session->send_seq)
- l2tp_dbg(session, L2TP_MSG_DATA, "%s: send %zd bytes, ns=%u\n",
- session->name, data_len, session->ns - 1);
- else
- l2tp_dbg(session, L2TP_MSG_DATA, "%s: send %zd bytes\n",
- session->name, data_len);
-
- if (session->debug & L2TP_MSG_DATA) {
- int uhlen = (tunnel->encap == L2TP_ENCAPTYPE_UDP) ? sizeof(struct udphdr) : 0;
- unsigned char *datap = skb->data + uhlen;
-
- pr_debug("%s: xmit\n", session->name);
- print_hex_dump_bytes("", DUMP_PREFIX_OFFSET,
- datap, min_t(size_t, 32, len - uhlen));
- }
-
/* Queue the packet to IP for output */
skb->ignore_df = 1;
skb_dst_drop(skb);
diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
index 7ed2b4eced94..657edad1263e 100644
--- a/net/l2tp/l2tp_eth.c
+++ b/net/l2tp/l2tp_eth.c
@@ -128,17 +128,6 @@ static void l2tp_eth_dev_recv(struct l2tp_session *session, struct sk_buff *skb,
struct net_device *dev;
struct l2tp_eth *priv;
- if (session->debug & L2TP_MSG_DATA) {
- unsigned int length;
-
- length = min(32u, skb->len);
- if (!pskb_may_pull(skb, length))
- goto error;
-
- pr_debug("%s: eth recv\n", session->name);
- print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, skb->data, length);
- }
-
if (!pskb_may_pull(skb, ETH_HLEN))
goto error;
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index df2a35b5714a..7086d97f293c 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -118,7 +118,6 @@ static int l2tp_ip_recv(struct sk_buff *skb)
struct l2tp_session *session;
struct l2tp_tunnel *tunnel = NULL;
struct iphdr *iph;
- int length;
if (!pskb_may_pull(skb, 4))
goto discard;
@@ -147,20 +146,6 @@ static int l2tp_ip_recv(struct sk_buff *skb)
if (!tunnel)
goto discard_sess;
- /* Trace packet contents, if enabled */
- if (tunnel->debug & L2TP_MSG_DATA) {
- length = min(32u, skb->len);
- if (!pskb_may_pull(skb, length))
- goto discard_sess;
-
- /* Point to L2TP header */
- optr = skb->data;
- ptr = skb->data;
- ptr += 4;
- pr_debug("%s: ip recv\n", tunnel->name);
- print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, ptr, length);
- }
-
if (l2tp_v3_ensure_opt_in_linear(session, skb, &ptr, &optr))
goto discard_sess;
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index bc757bc7e264..409ea8927f6c 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -131,7 +131,6 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
struct l2tp_session *session;
struct l2tp_tunnel *tunnel = NULL;
struct ipv6hdr *iph;
- int length;
if (!pskb_may_pull(skb, 4))
goto discard;
@@ -160,20 +159,6 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
if (!tunnel)
goto discard_sess;
- /* Trace packet contents, if enabled */
- if (tunnel->debug & L2TP_MSG_DATA) {
- length = min(32u, skb->len);
- if (!pskb_may_pull(skb, length))
- goto discard_sess;
-
- /* Point to L2TP header */
- optr = skb->data;
- ptr = skb->data;
- ptr += 4;
- pr_debug("%s: ip recv\n", tunnel->name);
- print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, ptr, length);
- }
-
if (l2tp_v3_ensure_opt_in_linear(session, skb, &ptr, &optr))
goto discard_sess;
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 13c3153b40d6..ee1663a3ca7b 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -237,17 +237,9 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
if (sk->sk_state & PPPOX_BOUND) {
struct pppox_sock *po;
- l2tp_dbg(session, L2TP_MSG_DATA,
- "%s: recv %d byte data frame, passing to ppp\n",
- session->name, data_len);
-
po = pppox_sk(sk);
ppp_input(&po->chan, skb);
} else {
- l2tp_dbg(session, L2TP_MSG_DATA,
- "%s: recv %d byte data frame, passing to L2TP socket\n",
- session->name, data_len);
-
if (sock_queue_rcv_skb(sk, skb) < 0) {
atomic_long_inc(&session->stats.rx_errors);
kfree_skb(skb);
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/9] l2tp: don't log data frames
2020-08-21 10:47 ` [PATCH 1/9] l2tp: don't log data frames Tom Parkin
@ 2020-08-21 12:28 ` kernel test robot
2020-08-21 16:33 ` Tom Parkin
0 siblings, 1 reply; 13+ messages in thread
From: kernel test robot @ 2020-08-21 12:28 UTC (permalink / raw)
To: Tom Parkin, netdev; +Cc: kbuild-all, jchapman, Tom Parkin
[-- Attachment #1: Type: text/plain, Size: 20513 bytes --]
Hi Tom,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tom-Parkin/l2tp-replace-custom-logging-code-with-tracepoints/20200821-184919
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git d0a84e1f38d9d6ae2dfab0b6c2407d667a265aa5
config: riscv-randconfig-r035-20200820 (attached as .config)
compiler: riscv32-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
net/l2tp/l2tp_core.c: In function 'l2tp_recv_common':
>> net/l2tp/l2tp_core.c:663:14: warning: variable 'nr' set but not used [-Wunused-but-set-variable]
663 | u32 ns = 0, nr = 0;
| ^~
# https://github.com/0day-ci/linux/commit/5b9d9c3057638c81876e600aa2210e4a3e35fa8d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tom-Parkin/l2tp-replace-custom-logging-code-with-tracepoints/20200821-184919
git checkout 5b9d9c3057638c81876e600aa2210e4a3e35fa8d
vim +/nr +663 net/l2tp/l2tp_core.c
b6dc01a43aaca24 James Chapman 2013-07-02 598
f7faffa3ff8ef6a James Chapman 2010-04-02 599 /* Do receive processing of L2TP data frames. We handle both L2TPv2
f7faffa3ff8ef6a James Chapman 2010-04-02 600 * and L2TPv3 data frames here.
f7faffa3ff8ef6a James Chapman 2010-04-02 601 *
f7faffa3ff8ef6a James Chapman 2010-04-02 602 * L2TPv2 Data Message Header
f7faffa3ff8ef6a James Chapman 2010-04-02 603 *
f7faffa3ff8ef6a James Chapman 2010-04-02 604 * 0 1 2 3
f7faffa3ff8ef6a James Chapman 2010-04-02 605 * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
f7faffa3ff8ef6a James Chapman 2010-04-02 606 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 607 * |T|L|x|x|S|x|O|P|x|x|x|x| Ver | Length (opt) |
f7faffa3ff8ef6a James Chapman 2010-04-02 608 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 609 * | Tunnel ID | Session ID |
f7faffa3ff8ef6a James Chapman 2010-04-02 610 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 611 * | Ns (opt) | Nr (opt) |
f7faffa3ff8ef6a James Chapman 2010-04-02 612 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 613 * | Offset Size (opt) | Offset pad... (opt)
f7faffa3ff8ef6a James Chapman 2010-04-02 614 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 615 *
f7faffa3ff8ef6a James Chapman 2010-04-02 616 * Data frames are marked by T=0. All other fields are the same as
f7faffa3ff8ef6a James Chapman 2010-04-02 617 * those in L2TP control frames.
f7faffa3ff8ef6a James Chapman 2010-04-02 618 *
f7faffa3ff8ef6a James Chapman 2010-04-02 619 * L2TPv3 Data Message Header
f7faffa3ff8ef6a James Chapman 2010-04-02 620 *
f7faffa3ff8ef6a James Chapman 2010-04-02 621 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 622 * | L2TP Session Header |
f7faffa3ff8ef6a James Chapman 2010-04-02 623 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 624 * | L2-Specific Sublayer |
f7faffa3ff8ef6a James Chapman 2010-04-02 625 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 626 * | Tunnel Payload ...
f7faffa3ff8ef6a James Chapman 2010-04-02 627 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 628 *
f7faffa3ff8ef6a James Chapman 2010-04-02 629 * L2TPv3 Session Header Over IP
f7faffa3ff8ef6a James Chapman 2010-04-02 630 *
f7faffa3ff8ef6a James Chapman 2010-04-02 631 * 0 1 2 3
f7faffa3ff8ef6a James Chapman 2010-04-02 632 * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
f7faffa3ff8ef6a James Chapman 2010-04-02 633 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 634 * | Session ID |
f7faffa3ff8ef6a James Chapman 2010-04-02 635 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 636 * | Cookie (optional, maximum 64 bits)...
f7faffa3ff8ef6a James Chapman 2010-04-02 637 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 638 * |
f7faffa3ff8ef6a James Chapman 2010-04-02 639 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 640 *
f7faffa3ff8ef6a James Chapman 2010-04-02 641 * L2TPv3 L2-Specific Sublayer Format
f7faffa3ff8ef6a James Chapman 2010-04-02 642 *
f7faffa3ff8ef6a James Chapman 2010-04-02 643 * 0 1 2 3
f7faffa3ff8ef6a James Chapman 2010-04-02 644 * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
f7faffa3ff8ef6a James Chapman 2010-04-02 645 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 646 * |x|S|x|x|x|x|x|x| Sequence Number |
f7faffa3ff8ef6a James Chapman 2010-04-02 647 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
f7faffa3ff8ef6a James Chapman 2010-04-02 648 *
23fe846f9a48d53 Guillaume Nault 2018-01-05 649 * Cookie value and sublayer format are negotiated with the peer when
23fe846f9a48d53 Guillaume Nault 2018-01-05 650 * the session is set up. Unlike L2TPv2, we do not need to parse the
23fe846f9a48d53 Guillaume Nault 2018-01-05 651 * packet header to determine if optional fields are present.
f7faffa3ff8ef6a James Chapman 2010-04-02 652 *
f7faffa3ff8ef6a James Chapman 2010-04-02 653 * Caller must already have parsed the frame and determined that it is
f7faffa3ff8ef6a James Chapman 2010-04-02 654 * a data (not control) frame before coming here. Fields up to the
f7faffa3ff8ef6a James Chapman 2010-04-02 655 * session-id have already been parsed and ptr points to the data
f7faffa3ff8ef6a James Chapman 2010-04-02 656 * after the session-id.
fd558d186df2c13 James Chapman 2010-04-02 657 */
f7faffa3ff8ef6a James Chapman 2010-04-02 658 void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
f7faffa3ff8ef6a James Chapman 2010-04-02 659 unsigned char *ptr, unsigned char *optr, u16 hdrflags,
2b139e6b1ec86e1 Guillaume Nault 2018-07-25 660 int length)
fd558d186df2c13 James Chapman 2010-04-02 661 {
f7faffa3ff8ef6a James Chapman 2010-04-02 662 struct l2tp_tunnel *tunnel = session->tunnel;
95075150d0bdaa7 Tom Parkin 2020-07-24 @663 u32 ns = 0, nr = 0;
fd558d186df2c13 James Chapman 2010-04-02 664 int offset;
fd558d186df2c13 James Chapman 2010-04-02 665
f7faffa3ff8ef6a James Chapman 2010-04-02 666 /* Parse and check optional cookie */
f7faffa3ff8ef6a James Chapman 2010-04-02 667 if (session->peer_cookie_len > 0) {
f7faffa3ff8ef6a James Chapman 2010-04-02 668 if (memcmp(ptr, &session->peer_cookie[0], session->peer_cookie_len)) {
a4ca44fa578c7c7 Joe Perches 2012-05-16 669 l2tp_info(tunnel, L2TP_MSG_DATA,
f7faffa3ff8ef6a James Chapman 2010-04-02 670 "%s: cookie mismatch (%u/%u). Discarding.\n",
a4ca44fa578c7c7 Joe Perches 2012-05-16 671 tunnel->name, tunnel->tunnel_id,
a4ca44fa578c7c7 Joe Perches 2012-05-16 672 session->session_id);
7b7c0719cd7afee Tom Parkin 2013-03-19 673 atomic_long_inc(&session->stats.rx_cookie_discards);
f7faffa3ff8ef6a James Chapman 2010-04-02 674 goto discard;
f7faffa3ff8ef6a James Chapman 2010-04-02 675 }
f7faffa3ff8ef6a James Chapman 2010-04-02 676 ptr += session->peer_cookie_len;
f7faffa3ff8ef6a James Chapman 2010-04-02 677 }
f7faffa3ff8ef6a James Chapman 2010-04-02 678
fd558d186df2c13 James Chapman 2010-04-02 679 /* Handle the optional sequence numbers. Sequence numbers are
fd558d186df2c13 James Chapman 2010-04-02 680 * in different places for L2TPv2 and L2TPv3.
fd558d186df2c13 James Chapman 2010-04-02 681 *
fd558d186df2c13 James Chapman 2010-04-02 682 * If we are the LAC, enable/disable sequence numbers under
fd558d186df2c13 James Chapman 2010-04-02 683 * the control of the LNS. If no sequence numbers present but
fd558d186df2c13 James Chapman 2010-04-02 684 * we were expecting them, discard frame.
fd558d186df2c13 James Chapman 2010-04-02 685 */
fd558d186df2c13 James Chapman 2010-04-02 686 L2TP_SKB_CB(skb)->has_seq = 0;
f7faffa3ff8ef6a James Chapman 2010-04-02 687 if (tunnel->version == L2TP_HDR_VER_2) {
fd558d186df2c13 James Chapman 2010-04-02 688 if (hdrflags & L2TP_HDRFLAG_S) {
f7faffa3ff8ef6a James Chapman 2010-04-02 689 ns = ntohs(*(__be16 *)ptr);
fd558d186df2c13 James Chapman 2010-04-02 690 ptr += 2;
fd558d186df2c13 James Chapman 2010-04-02 691 nr = ntohs(*(__be16 *)ptr);
fd558d186df2c13 James Chapman 2010-04-02 692 ptr += 2;
fd558d186df2c13 James Chapman 2010-04-02 693
fd558d186df2c13 James Chapman 2010-04-02 694 /* Store L2TP info in the skb */
fd558d186df2c13 James Chapman 2010-04-02 695 L2TP_SKB_CB(skb)->ns = ns;
fd558d186df2c13 James Chapman 2010-04-02 696 L2TP_SKB_CB(skb)->has_seq = 1;
fd558d186df2c13 James Chapman 2010-04-02 697 }
f7faffa3ff8ef6a James Chapman 2010-04-02 698 } else if (session->l2specific_type == L2TP_L2SPECTYPE_DEFAULT) {
f7faffa3ff8ef6a James Chapman 2010-04-02 699 u32 l2h = ntohl(*(__be32 *)ptr);
f7faffa3ff8ef6a James Chapman 2010-04-02 700
f7faffa3ff8ef6a James Chapman 2010-04-02 701 if (l2h & 0x40000000) {
f7faffa3ff8ef6a James Chapman 2010-04-02 702 ns = l2h & 0x00ffffff;
f7faffa3ff8ef6a James Chapman 2010-04-02 703
f7faffa3ff8ef6a James Chapman 2010-04-02 704 /* Store L2TP info in the skb */
f7faffa3ff8ef6a James Chapman 2010-04-02 705 L2TP_SKB_CB(skb)->ns = ns;
f7faffa3ff8ef6a James Chapman 2010-04-02 706 L2TP_SKB_CB(skb)->has_seq = 1;
f7faffa3ff8ef6a James Chapman 2010-04-02 707 }
62e7b6a57c7b9bf Lorenzo Bianconi 2018-01-16 708 ptr += 4;
f7faffa3ff8ef6a James Chapman 2010-04-02 709 }
f7faffa3ff8ef6a James Chapman 2010-04-02 710
fd558d186df2c13 James Chapman 2010-04-02 711 if (L2TP_SKB_CB(skb)->has_seq) {
20dcb1107ab1a34 Tom Parkin 2020-07-22 712 /* Received a packet with sequence numbers. If we're the LAC,
fd558d186df2c13 James Chapman 2010-04-02 713 * check if we sre sending sequence numbers and if not,
fd558d186df2c13 James Chapman 2010-04-02 714 * configure it so.
fd558d186df2c13 James Chapman 2010-04-02 715 */
6c0ec37b8283463 Tom Parkin 2020-07-23 716 if (!session->lns_mode && !session->send_seq) {
a4ca44fa578c7c7 Joe Perches 2012-05-16 717 l2tp_info(session, L2TP_MSG_SEQ,
fd558d186df2c13 James Chapman 2010-04-02 718 "%s: requested to enable seq numbers by LNS\n",
fd558d186df2c13 James Chapman 2010-04-02 719 session->name);
3f9b9770b479986 Asbjørn Sloth Tønnesen 2016-11-07 720 session->send_seq = 1;
f7faffa3ff8ef6a James Chapman 2010-04-02 721 l2tp_session_set_header_len(session, tunnel->version);
fd558d186df2c13 James Chapman 2010-04-02 722 }
fd558d186df2c13 James Chapman 2010-04-02 723 } else {
fd558d186df2c13 James Chapman 2010-04-02 724 /* No sequence numbers.
fd558d186df2c13 James Chapman 2010-04-02 725 * If user has configured mandatory sequence numbers, discard.
fd558d186df2c13 James Chapman 2010-04-02 726 */
fd558d186df2c13 James Chapman 2010-04-02 727 if (session->recv_seq) {
a4ca44fa578c7c7 Joe Perches 2012-05-16 728 l2tp_warn(session, L2TP_MSG_SEQ,
a4ca44fa578c7c7 Joe Perches 2012-05-16 729 "%s: recv data has no seq numbers when required. Discarding.\n",
a4ca44fa578c7c7 Joe Perches 2012-05-16 730 session->name);
7b7c0719cd7afee Tom Parkin 2013-03-19 731 atomic_long_inc(&session->stats.rx_seq_discards);
fd558d186df2c13 James Chapman 2010-04-02 732 goto discard;
fd558d186df2c13 James Chapman 2010-04-02 733 }
fd558d186df2c13 James Chapman 2010-04-02 734
fd558d186df2c13 James Chapman 2010-04-02 735 /* If we're the LAC and we're sending sequence numbers, the
fd558d186df2c13 James Chapman 2010-04-02 736 * LNS has requested that we no longer send sequence numbers.
fd558d186df2c13 James Chapman 2010-04-02 737 * If we're the LNS and we're sending sequence numbers, the
fd558d186df2c13 James Chapman 2010-04-02 738 * LAC is broken. Discard the frame.
fd558d186df2c13 James Chapman 2010-04-02 739 */
6c0ec37b8283463 Tom Parkin 2020-07-23 740 if (!session->lns_mode && session->send_seq) {
a4ca44fa578c7c7 Joe Perches 2012-05-16 741 l2tp_info(session, L2TP_MSG_SEQ,
fd558d186df2c13 James Chapman 2010-04-02 742 "%s: requested to disable seq numbers by LNS\n",
fd558d186df2c13 James Chapman 2010-04-02 743 session->name);
fd558d186df2c13 James Chapman 2010-04-02 744 session->send_seq = 0;
f7faffa3ff8ef6a James Chapman 2010-04-02 745 l2tp_session_set_header_len(session, tunnel->version);
fd558d186df2c13 James Chapman 2010-04-02 746 } else if (session->send_seq) {
a4ca44fa578c7c7 Joe Perches 2012-05-16 747 l2tp_warn(session, L2TP_MSG_SEQ,
a4ca44fa578c7c7 Joe Perches 2012-05-16 748 "%s: recv data has no seq numbers when required. Discarding.\n",
a4ca44fa578c7c7 Joe Perches 2012-05-16 749 session->name);
7b7c0719cd7afee Tom Parkin 2013-03-19 750 atomic_long_inc(&session->stats.rx_seq_discards);
fd558d186df2c13 James Chapman 2010-04-02 751 goto discard;
fd558d186df2c13 James Chapman 2010-04-02 752 }
fd558d186df2c13 James Chapman 2010-04-02 753 }
fd558d186df2c13 James Chapman 2010-04-02 754
900631ee6a2651d James Chapman 2018-01-03 755 /* Session data offset is defined only for L2TPv2 and is
900631ee6a2651d James Chapman 2018-01-03 756 * indicated by an optional 16-bit value in the header.
f7faffa3ff8ef6a James Chapman 2010-04-02 757 */
f7faffa3ff8ef6a James Chapman 2010-04-02 758 if (tunnel->version == L2TP_HDR_VER_2) {
fd558d186df2c13 James Chapman 2010-04-02 759 /* If offset bit set, skip it. */
fd558d186df2c13 James Chapman 2010-04-02 760 if (hdrflags & L2TP_HDRFLAG_O) {
fd558d186df2c13 James Chapman 2010-04-02 761 offset = ntohs(*(__be16 *)ptr);
fd558d186df2c13 James Chapman 2010-04-02 762 ptr += 2 + offset;
fd558d186df2c13 James Chapman 2010-04-02 763 }
900631ee6a2651d James Chapman 2018-01-03 764 }
fd558d186df2c13 James Chapman 2010-04-02 765
fd558d186df2c13 James Chapman 2010-04-02 766 offset = ptr - optr;
fd558d186df2c13 James Chapman 2010-04-02 767 if (!pskb_may_pull(skb, offset))
fd558d186df2c13 James Chapman 2010-04-02 768 goto discard;
fd558d186df2c13 James Chapman 2010-04-02 769
fd558d186df2c13 James Chapman 2010-04-02 770 __skb_pull(skb, offset);
fd558d186df2c13 James Chapman 2010-04-02 771
fd558d186df2c13 James Chapman 2010-04-02 772 /* Prepare skb for adding to the session's reorder_q. Hold
fd558d186df2c13 James Chapman 2010-04-02 773 * packets for max reorder_timeout or 1 second if not
fd558d186df2c13 James Chapman 2010-04-02 774 * reordering.
fd558d186df2c13 James Chapman 2010-04-02 775 */
fd558d186df2c13 James Chapman 2010-04-02 776 L2TP_SKB_CB(skb)->length = length;
fd558d186df2c13 James Chapman 2010-04-02 777 L2TP_SKB_CB(skb)->expires = jiffies +
fd558d186df2c13 James Chapman 2010-04-02 778 (session->reorder_timeout ? session->reorder_timeout : HZ);
fd558d186df2c13 James Chapman 2010-04-02 779
fd558d186df2c13 James Chapman 2010-04-02 780 /* Add packet to the session's receive queue. Reordering is done here, if
fd558d186df2c13 James Chapman 2010-04-02 781 * enabled. Saved L2TP protocol info is stored in skb->sb[].
fd558d186df2c13 James Chapman 2010-04-02 782 */
fd558d186df2c13 James Chapman 2010-04-02 783 if (L2TP_SKB_CB(skb)->has_seq) {
b6dc01a43aaca24 James Chapman 2013-07-02 784 if (l2tp_recv_data_seq(session, skb))
fd558d186df2c13 James Chapman 2010-04-02 785 goto discard;
fd558d186df2c13 James Chapman 2010-04-02 786 } else {
fd558d186df2c13 James Chapman 2010-04-02 787 /* No sequence numbers. Add the skb to the tail of the
fd558d186df2c13 James Chapman 2010-04-02 788 * reorder queue. This ensures that it will be
fd558d186df2c13 James Chapman 2010-04-02 789 * delivered after all previous sequenced skbs.
fd558d186df2c13 James Chapman 2010-04-02 790 */
fd558d186df2c13 James Chapman 2010-04-02 791 skb_queue_tail(&session->reorder_q, skb);
fd558d186df2c13 James Chapman 2010-04-02 792 }
fd558d186df2c13 James Chapman 2010-04-02 793
fd558d186df2c13 James Chapman 2010-04-02 794 /* Try to dequeue as many skbs from reorder_q as we can. */
fd558d186df2c13 James Chapman 2010-04-02 795 l2tp_recv_dequeue(session);
fd558d186df2c13 James Chapman 2010-04-02 796
f7faffa3ff8ef6a James Chapman 2010-04-02 797 return;
fd558d186df2c13 James Chapman 2010-04-02 798
fd558d186df2c13 James Chapman 2010-04-02 799 discard:
7b7c0719cd7afee Tom Parkin 2013-03-19 800 atomic_long_inc(&session->stats.rx_errors);
fd558d186df2c13 James Chapman 2010-04-02 801 kfree_skb(skb);
f7faffa3ff8ef6a James Chapman 2010-04-02 802 }
ca7885dbcd899e6 Tom Parkin 2020-07-28 803 EXPORT_SYMBOL_GPL(l2tp_recv_common);
f7faffa3ff8ef6a James Chapman 2010-04-02 804
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25492 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/9] l2tp: don't log data frames
2020-08-21 12:28 ` kernel test robot
@ 2020-08-21 16:33 ` Tom Parkin
0 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 16:33 UTC (permalink / raw)
To: kernel test robot; +Cc: netdev, kbuild-all, jchapman
[-- Attachment #1: Type: text/plain, Size: 533 bytes --]
On Fri, Aug 21, 2020 at 20:28:59 +0800, kernel test robot wrote:
> Hi Tom,
>
> Thank you for the patch! Perhaps something to improve:
>
[snip]
>
> net/l2tp/l2tp_core.c: In function 'l2tp_recv_common':
> >> net/l2tp/l2tp_core.c:663:14: warning: variable 'nr' set but not used [-Wunused-but-set-variable]
> 663 | u32 ns = 0, nr = 0;
> | ^
Thank you kernel test robot!
I will incorporate a fix for this in a v2 patchset, but I'll wait a
little while for any further review comments.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/9] l2tp: remove noisy logging, use appropriate log levels
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
2020-08-21 10:47 ` [PATCH 1/9] l2tp: don't log data frames Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 10:47 ` [PATCH 3/9] l2tp: use standard API for warning log messages Tom Parkin
` (7 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
l2tp_ppp in particular had a lot of log messages for tracing
[get|set]sockopt calls. These aren't especially useful, so remove
these messages.
Several log messages flagging error conditions were logged using
l2tp_info: they're better off as l2tp_warn.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
net/l2tp/l2tp_core.c | 16 ++++------------
net/l2tp/l2tp_ppp.c | 32 +-------------------------------
2 files changed, 5 insertions(+), 43 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index ce647816da61..53a5556699f8 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -666,7 +666,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
/* Parse and check optional cookie */
if (session->peer_cookie_len > 0) {
if (memcmp(ptr, &session->peer_cookie[0], session->peer_cookie_len)) {
- l2tp_info(tunnel, L2TP_MSG_DATA,
+ l2tp_warn(tunnel, L2TP_MSG_DATA,
"%s: cookie mismatch (%u/%u). Discarding.\n",
tunnel->name, tunnel->tunnel_id,
session->session_id);
@@ -839,7 +839,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
/* Short packet? */
if (!pskb_may_pull(skb, L2TP_HDR_SIZE_MAX)) {
- l2tp_info(tunnel, L2TP_MSG_DATA,
+ l2tp_warn(tunnel, L2TP_MSG_DATA,
"%s: recv short packet (len=%d)\n",
tunnel->name, skb->len);
goto error;
@@ -855,7 +855,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
/* Check protocol version */
version = hdrflags & L2TP_HDR_VER_MASK;
if (version != tunnel->version) {
- l2tp_info(tunnel, L2TP_MSG_DATA,
+ l2tp_warn(tunnel, L2TP_MSG_DATA,
"%s: recv protocol version mismatch: got %d expected %d\n",
tunnel->name, version, tunnel->version);
goto error;
@@ -895,7 +895,7 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
l2tp_session_dec_refcount(session);
/* Not found? Pass to userspace to deal with */
- l2tp_info(tunnel, L2TP_MSG_DATA,
+ l2tp_warn(tunnel, L2TP_MSG_DATA,
"%s: no session found (%u/%u). Passing up.\n",
tunnel->name, tunnel_id, session_id);
goto error;
@@ -1153,8 +1153,6 @@ static void l2tp_tunnel_destruct(struct sock *sk)
if (!tunnel)
goto end;
- l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: closing...\n", tunnel->name);
-
/* Disable udp encapsulation */
switch (tunnel->encap) {
case L2TP_ENCAPTYPE_UDP:
@@ -1213,9 +1211,6 @@ static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
struct hlist_node *tmp;
struct l2tp_session *session;
- l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: closing all sessions...\n",
- tunnel->name);
-
write_lock_bh(&tunnel->hlist_lock);
tunnel->acpt_newsess = false;
for (hash = 0; hash < L2TP_HASH_SIZE; hash++) {
@@ -1223,9 +1218,6 @@ static void l2tp_tunnel_closeall(struct l2tp_tunnel *tunnel)
hlist_for_each_safe(walk, tmp, &tunnel->session_hlist[hash]) {
session = hlist_entry(walk, struct l2tp_session, hlist);
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: closing session\n", session->name);
-
hlist_del_init(&session->hlist);
if (test_and_set_bit(0, &session->dead))
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index ee1663a3ca7b..660ea95e0910 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -251,7 +251,7 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
no_sock:
rcu_read_unlock();
- l2tp_info(session, L2TP_MSG_DATA, "%s: no socket\n", session->name);
+ l2tp_warn(session, L2TP_MSG_DATA, "%s: no socket\n", session->name);
kfree_skb(skb);
}
@@ -832,8 +832,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
drop_refcnt = false;
sk->sk_state = PPPOX_CONNECTED;
- l2tp_info(session, L2TP_MSG_CONTROL, "%s: created\n",
- session->name);
end:
if (error) {
@@ -1150,8 +1148,6 @@ static int pppol2tp_tunnel_setsockopt(struct sock *sk,
switch (optname) {
case PPPOL2TP_SO_DEBUG:
tunnel->debug = val;
- l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: set debug=%x\n",
- tunnel->name, tunnel->debug);
break;
default:
@@ -1177,9 +1173,6 @@ static int pppol2tp_session_setsockopt(struct sock *sk,
break;
}
session->recv_seq = !!val;
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: set recv_seq=%d\n",
- session->name, session->recv_seq);
break;
case PPPOL2TP_SO_SENDSEQ:
@@ -1195,9 +1188,6 @@ static int pppol2tp_session_setsockopt(struct sock *sk,
PPPOL2TP_L2TP_HDR_SIZE_NOSEQ;
}
l2tp_session_set_header_len(session, session->tunnel->version);
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: set send_seq=%d\n",
- session->name, session->send_seq);
break;
case PPPOL2TP_SO_LNSMODE:
@@ -1206,22 +1196,14 @@ static int pppol2tp_session_setsockopt(struct sock *sk,
break;
}
session->lns_mode = !!val;
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: set lns_mode=%d\n",
- session->name, session->lns_mode);
break;
case PPPOL2TP_SO_DEBUG:
session->debug = val;
- l2tp_info(session, L2TP_MSG_CONTROL, "%s: set debug=%x\n",
- session->name, session->debug);
break;
case PPPOL2TP_SO_REORDERTO:
session->reorder_timeout = msecs_to_jiffies(val);
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: set reorder_timeout=%d\n",
- session->name, session->reorder_timeout);
break;
default:
@@ -1290,8 +1272,6 @@ static int pppol2tp_tunnel_getsockopt(struct sock *sk,
switch (optname) {
case PPPOL2TP_SO_DEBUG:
*val = tunnel->debug;
- l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: get debug=%x\n",
- tunnel->name, tunnel->debug);
break;
default:
@@ -1313,32 +1293,22 @@ static int pppol2tp_session_getsockopt(struct sock *sk,
switch (optname) {
case PPPOL2TP_SO_RECVSEQ:
*val = session->recv_seq;
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: get recv_seq=%d\n", session->name, *val);
break;
case PPPOL2TP_SO_SENDSEQ:
*val = session->send_seq;
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: get send_seq=%d\n", session->name, *val);
break;
case PPPOL2TP_SO_LNSMODE:
*val = session->lns_mode;
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: get lns_mode=%d\n", session->name, *val);
break;
case PPPOL2TP_SO_DEBUG:
*val = session->debug;
- l2tp_info(session, L2TP_MSG_CONTROL, "%s: get debug=%d\n",
- session->name, *val);
break;
case PPPOL2TP_SO_REORDERTO:
*val = (int)jiffies_to_msecs(session->reorder_timeout);
- l2tp_info(session, L2TP_MSG_CONTROL,
- "%s: get reorder_timeout=%d\n", session->name, *val);
break;
default:
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/9] l2tp: use standard API for warning log messages
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
2020-08-21 10:47 ` [PATCH 1/9] l2tp: don't log data frames Tom Parkin
2020-08-21 10:47 ` [PATCH 2/9] l2tp: remove noisy logging, use appropriate log levels Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 10:47 ` [PATCH 4/9] l2tp: add tracepoint infrastructure to core Tom Parkin
` (6 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
The l2tp_* log wrappers only emit messages of a given category if the
tunnel or session structure has the appropriate flag set in its debug
field. Flags default to being unset.
For warning messages, this doesn't make a lot of sense since an
administrator is likely to want to know about datapath warnings without
needing to tweak the debug flags setting for a given tunnel or session
instance.
Modify l2tp_warn callsites to use pr_warn_ratelimited instead for
unconditional output of warning messages.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
net/l2tp/l2tp_core.c | 32 +++++++++++++-------------------
net/l2tp/l2tp_ppp.c | 2 +-
2 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 53a5556699f8..a0f982add6ad 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -666,10 +666,9 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
/* Parse and check optional cookie */
if (session->peer_cookie_len > 0) {
if (memcmp(ptr, &session->peer_cookie[0], session->peer_cookie_len)) {
- l2tp_warn(tunnel, L2TP_MSG_DATA,
- "%s: cookie mismatch (%u/%u). Discarding.\n",
- tunnel->name, tunnel->tunnel_id,
- session->session_id);
+ pr_warn_ratelimited("%s: cookie mismatch (%u/%u). Discarding.\n",
+ tunnel->name, tunnel->tunnel_id,
+ session->session_id);
atomic_long_inc(&session->stats.rx_cookie_discards);
goto discard;
}
@@ -725,9 +724,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
* If user has configured mandatory sequence numbers, discard.
*/
if (session->recv_seq) {
- l2tp_warn(session, L2TP_MSG_SEQ,
- "%s: recv data has no seq numbers when required. Discarding.\n",
- session->name);
+ pr_warn_ratelimited("%s: recv data has no seq numbers when required. Discarding.\n",
+ session->name);
atomic_long_inc(&session->stats.rx_seq_discards);
goto discard;
}
@@ -744,9 +742,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
session->send_seq = 0;
l2tp_session_set_header_len(session, tunnel->version);
} else if (session->send_seq) {
- l2tp_warn(session, L2TP_MSG_SEQ,
- "%s: recv data has no seq numbers when required. Discarding.\n",
- session->name);
+ pr_warn_ratelimited("%s: recv data has no seq numbers when required. Discarding.\n",
+ session->name);
atomic_long_inc(&session->stats.rx_seq_discards);
goto discard;
}
@@ -839,9 +836,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
/* Short packet? */
if (!pskb_may_pull(skb, L2TP_HDR_SIZE_MAX)) {
- l2tp_warn(tunnel, L2TP_MSG_DATA,
- "%s: recv short packet (len=%d)\n",
- tunnel->name, skb->len);
+ pr_warn_ratelimited("%s: recv short packet (len=%d)\n",
+ tunnel->name, skb->len);
goto error;
}
@@ -855,9 +851,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
/* Check protocol version */
version = hdrflags & L2TP_HDR_VER_MASK;
if (version != tunnel->version) {
- l2tp_warn(tunnel, L2TP_MSG_DATA,
- "%s: recv protocol version mismatch: got %d expected %d\n",
- tunnel->name, version, tunnel->version);
+ pr_warn_ratelimited("%s: recv protocol version mismatch: got %d expected %d\n",
+ tunnel->name, version, tunnel->version);
goto error;
}
@@ -895,9 +890,8 @@ static int l2tp_udp_recv_core(struct l2tp_tunnel *tunnel, struct sk_buff *skb)
l2tp_session_dec_refcount(session);
/* Not found? Pass to userspace to deal with */
- l2tp_warn(tunnel, L2TP_MSG_DATA,
- "%s: no session found (%u/%u). Passing up.\n",
- tunnel->name, tunnel_id, session_id);
+ pr_warn_ratelimited("%s: no session found (%u/%u). Passing up.\n",
+ tunnel->name, tunnel_id, session_id);
goto error;
}
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 660ea95e0910..bd6bb17dfadb 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -251,7 +251,7 @@ static void pppol2tp_recv(struct l2tp_session *session, struct sk_buff *skb, int
no_sock:
rcu_read_unlock();
- l2tp_warn(session, L2TP_MSG_DATA, "%s: no socket\n", session->name);
+ pr_warn_ratelimited("%s: no socket in recv\n", session->name);
kfree_skb(skb);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/9] l2tp: add tracepoint infrastructure to core
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
` (2 preceding siblings ...)
2020-08-21 10:47 ` [PATCH 3/9] l2tp: use standard API for warning log messages Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 10:47 ` [PATCH 5/9] l2tp: add tracepoint definitions in trace.h Tom Parkin
` (5 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
The l2tp subsystem doesn't currently make use of tracepoints.
As a starting point for adding tracepoints, add skeleton infrastructure
for defining tracepoints for the subsystem, and for having them build
appropriately whether compiled into the kernel or built as a module.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
net/l2tp/Makefile | 2 ++
net/l2tp/l2tp_core.c | 3 +++
net/l2tp/trace.h | 15 +++++++++++++++
3 files changed, 20 insertions(+)
create mode 100644 net/l2tp/trace.h
diff --git a/net/l2tp/Makefile b/net/l2tp/Makefile
index 399a7e5db2f4..cf8f27071d3f 100644
--- a/net/l2tp/Makefile
+++ b/net/l2tp/Makefile
@@ -5,6 +5,8 @@
obj-$(CONFIG_L2TP) += l2tp_core.o
+CFLAGS_l2tp_core.o += -I$(src)
+
# Build l2tp as modules if L2TP is M
obj-$(subst y,$(CONFIG_L2TP),$(CONFIG_PPPOL2TP)) += l2tp_ppp.o
obj-$(subst y,$(CONFIG_L2TP),$(CONFIG_L2TP_IP)) += l2tp_ip.o
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index a0f982add6ad..a9825724e2f4 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -62,6 +62,9 @@
#include "l2tp_core.h"
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
#define L2TP_DRV_VERSION "V2.0"
/* L2TP header constants */
diff --git a/net/l2tp/trace.h b/net/l2tp/trace.h
new file mode 100644
index 000000000000..652778291b77
--- /dev/null
+++ b/net/l2tp/trace.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM l2tp
+
+#if !defined(_TRACE_L2TP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_L2TP_H
+
+#endif /* _TRACE_L2TP_H */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE trace
+#include <trace/define_trace.h>
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/9] l2tp: add tracepoint definitions in trace.h
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
` (3 preceding siblings ...)
2020-08-21 10:47 ` [PATCH 4/9] l2tp: add tracepoint infrastructure to core Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 10:47 ` [PATCH 6/9] l2tp: add tracepoints to l2tp_core.c Tom Parkin
` (4 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
l2tp can provide a better debug experience using tracepoints rather than
printk-style logging.
Add tracepoint definitions in trace.h for use in the l2tp subsystem
code.
Add preprocessor definitions for the length of session and tunnel names
in l2tp_core.h so we can reuse these in trace.h.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
net/l2tp/l2tp_core.h | 6 +-
net/l2tp/trace.h | 196 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 200 insertions(+), 2 deletions(-)
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 3468d6b177a0..835c4645a0ec 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -66,6 +66,7 @@ struct l2tp_session_cfg {
* Is linked into a per-tunnel session hashlist; and in the case of an L2TPv3 session into
* an additional per-net ("global") hashlist.
*/
+#define L2TP_SESSION_NAME_MAX 32
struct l2tp_session {
int magic; /* should be L2TP_SESSION_MAGIC */
long dead;
@@ -90,7 +91,7 @@ struct l2tp_session {
struct hlist_node hlist; /* hash list node */
refcount_t ref_count;
- char name[32]; /* for logging */
+ char name[L2TP_SESSION_NAME_MAX]; /* for logging */
char ifname[IFNAMSIZ];
unsigned int recv_seq:1; /* expect receive packets with sequence numbers? */
unsigned int send_seq:1; /* send packets with sequence numbers? */
@@ -154,6 +155,7 @@ struct l2tp_tunnel_cfg {
* Maintains a hashlist of sessions belonging to the tunnel instance.
* Is linked into a per-net list of tunnels.
*/
+#define L2TP_TUNNEL_NAME_MAX 20
struct l2tp_tunnel {
int magic; /* Should be L2TP_TUNNEL_MAGIC */
@@ -170,7 +172,7 @@ struct l2tp_tunnel {
u32 peer_tunnel_id;
int version; /* 2=>L2TPv2, 3=>L2TPv3 */
- char name[20]; /* for logging */
+ char name[L2TP_TUNNEL_NAME_MAX]; /* for logging */
int debug; /* bitmask of debug message categories */
enum l2tp_encap_type encap;
struct l2tp_stats stats;
diff --git a/net/l2tp/trace.h b/net/l2tp/trace.h
index 652778291b77..8596eaa12a2e 100644
--- a/net/l2tp/trace.h
+++ b/net/l2tp/trace.h
@@ -5,6 +5,202 @@
#if !defined(_TRACE_L2TP_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_L2TP_H
+#include <linux/tracepoint.h>
+#include <linux/l2tp.h>
+#include "l2tp_core.h"
+
+#define encap_type_name(e) { L2TP_ENCAPTYPE_##e, #e }
+#define show_encap_type_name(val) \
+ __print_symbolic(val, \
+ encap_type_name(UDP), \
+ encap_type_name(IP))
+
+#define pw_type_name(p) { L2TP_PWTYPE_##p, #p }
+#define show_pw_type_name(val) \
+ __print_symbolic(val, \
+ pw_type_name(ETH_VLAN), \
+ pw_type_name(ETH), \
+ pw_type_name(PPP), \
+ pw_type_name(PPP_AC), \
+ pw_type_name(IP))
+
+DECLARE_EVENT_CLASS(tunnel_only_evt,
+ TP_PROTO(struct l2tp_tunnel *tunnel),
+ TP_ARGS(tunnel),
+ TP_STRUCT__entry(
+ __array(char, name, L2TP_TUNNEL_NAME_MAX)
+ ),
+ TP_fast_assign(
+ memcpy(__entry->name, tunnel->name, L2TP_TUNNEL_NAME_MAX);
+ ),
+ TP_printk("%s", __entry->name)
+);
+
+DECLARE_EVENT_CLASS(session_only_evt,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session),
+ TP_STRUCT__entry(
+ __array(char, name, L2TP_SESSION_NAME_MAX)
+ ),
+ TP_fast_assign(
+ memcpy(__entry->name, session->name, L2TP_SESSION_NAME_MAX);
+ ),
+ TP_printk("%s", __entry->name)
+);
+
+TRACE_EVENT(register_tunnel,
+ TP_PROTO(struct l2tp_tunnel *tunnel),
+ TP_ARGS(tunnel),
+ TP_STRUCT__entry(
+ __array(char, name, L2TP_TUNNEL_NAME_MAX)
+ __field(int, fd)
+ __field(u32, tid)
+ __field(u32, ptid)
+ __field(int, version)
+ __field(enum l2tp_encap_type, encap)
+ ),
+ TP_fast_assign(
+ memcpy(__entry->name, tunnel->name, L2TP_TUNNEL_NAME_MAX);
+ __entry->fd = tunnel->fd;
+ __entry->tid = tunnel->tunnel_id;
+ __entry->ptid = tunnel->peer_tunnel_id;
+ __entry->version = tunnel->version;
+ __entry->encap = tunnel->encap;
+ ),
+ TP_printk("%s: type=%s encap=%s version=L2TPv%d tid=%u ptid=%u fd=%d",
+ __entry->name,
+ __entry->fd > 0 ? "managed" : "unmanaged",
+ show_encap_type_name(__entry->encap),
+ __entry->version,
+ __entry->tid,
+ __entry->ptid,
+ __entry->fd)
+);
+
+DEFINE_EVENT(tunnel_only_evt, delete_tunnel,
+ TP_PROTO(struct l2tp_tunnel *tunnel),
+ TP_ARGS(tunnel)
+);
+
+DEFINE_EVENT(tunnel_only_evt, free_tunnel,
+ TP_PROTO(struct l2tp_tunnel *tunnel),
+ TP_ARGS(tunnel)
+);
+
+TRACE_EVENT(register_session,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session),
+ TP_STRUCT__entry(
+ __array(char, name, L2TP_SESSION_NAME_MAX)
+ __field(u32, tid)
+ __field(u32, ptid)
+ __field(u32, sid)
+ __field(u32, psid)
+ __field(enum l2tp_pwtype, pwtype)
+ ),
+ TP_fast_assign(
+ memcpy(__entry->name, session->name, L2TP_SESSION_NAME_MAX);
+ __entry->tid = session->tunnel ? session->tunnel->tunnel_id : 0;
+ __entry->ptid = session->tunnel ? session->tunnel->peer_tunnel_id : 0;
+ __entry->sid = session->session_id;
+ __entry->psid = session->peer_session_id;
+ __entry->pwtype = session->pwtype;
+ ),
+ TP_printk("%s: pseudowire=%s sid=%u psid=%u tid=%u ptid=%u",
+ __entry->name,
+ show_pw_type_name(__entry->pwtype),
+ __entry->sid,
+ __entry->psid,
+ __entry->sid,
+ __entry->psid)
+);
+
+DEFINE_EVENT(session_only_evt, delete_session,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session)
+);
+
+DEFINE_EVENT(session_only_evt, free_session,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session)
+);
+
+DEFINE_EVENT(session_only_evt, session_seqnum_lns_enable,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session)
+);
+
+DEFINE_EVENT(session_only_evt, session_seqnum_lns_disable,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session)
+);
+
+DECLARE_EVENT_CLASS(session_seqnum_evt,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session),
+ TP_STRUCT__entry(
+ __array(char, name, L2TP_SESSION_NAME_MAX)
+ __field(u32, ns)
+ __field(u32, nr)
+ ),
+ TP_fast_assign(
+ memcpy(__entry->name, session->name, L2TP_SESSION_NAME_MAX);
+ __entry->ns = session->ns;
+ __entry->nr = session->nr;
+ ),
+ TP_printk("%s: ns=%u nr=%u",
+ __entry->name,
+ __entry->ns,
+ __entry->nr)
+);
+
+DEFINE_EVENT(session_seqnum_evt, session_seqnum_update,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session)
+);
+
+DEFINE_EVENT(session_seqnum_evt, session_seqnum_reset,
+ TP_PROTO(struct l2tp_session *session),
+ TP_ARGS(session)
+);
+
+DECLARE_EVENT_CLASS(session_pkt_discard_evt,
+ TP_PROTO(struct l2tp_session *session, u32 pkt_ns),
+ TP_ARGS(session, pkt_ns),
+ TP_STRUCT__entry(
+ __array(char, name, L2TP_SESSION_NAME_MAX)
+ __field(u32, pkt_ns)
+ __field(u32, my_nr)
+ __field(u32, reorder_q_len)
+ ),
+ TP_fast_assign(
+ memcpy(__entry->name, session->name, L2TP_SESSION_NAME_MAX);
+ __entry->pkt_ns = pkt_ns,
+ __entry->my_nr = session->nr;
+ __entry->reorder_q_len = skb_queue_len(&session->reorder_q);
+ ),
+ TP_printk("%s: pkt_ns=%u my_nr=%u reorder_q_len=%u",
+ __entry->name,
+ __entry->pkt_ns,
+ __entry->my_nr,
+ __entry->reorder_q_len)
+);
+
+DEFINE_EVENT(session_pkt_discard_evt, session_pkt_expired,
+ TP_PROTO(struct l2tp_session *session, u32 pkt_ns),
+ TP_ARGS(session, pkt_ns)
+);
+
+DEFINE_EVENT(session_pkt_discard_evt, session_pkt_outside_rx_window,
+ TP_PROTO(struct l2tp_session *session, u32 pkt_ns),
+ TP_ARGS(session, pkt_ns)
+);
+
+DEFINE_EVENT(session_pkt_discard_evt, session_pkt_oos,
+ TP_PROTO(struct l2tp_session *session, u32 pkt_ns),
+ TP_ARGS(session, pkt_ns)
+);
+
#endif /* _TRACE_L2TP_H */
/* This part must be outside protection */
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 6/9] l2tp: add tracepoints to l2tp_core.c
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
` (4 preceding siblings ...)
2020-08-21 10:47 ` [PATCH 5/9] l2tp: add tracepoint definitions in trace.h Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 10:47 ` [PATCH 7/9] l2tp: remove custom logging macros Tom Parkin
` (3 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
Add lifetime event tracing for tunnel and session instances, tracking
tunnel and session registration, deletion, and eventual freeing.
Port the data path sequence number debug logging to use trace points
rather than custom debug macros.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
net/l2tp/l2tp_core.c | 83 +++++++++++++++++---------------------------
1 file changed, 31 insertions(+), 52 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index a9825724e2f4..651c08dc9bcf 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -61,6 +61,7 @@
#include <linux/atomic.h>
#include "l2tp_core.h"
+#include "trace.h"
#define CREATE_TRACE_POINTS
#include "trace.h"
@@ -154,6 +155,7 @@ l2tp_session_id_hash(struct l2tp_tunnel *tunnel, u32 session_id)
static void l2tp_tunnel_free(struct l2tp_tunnel *tunnel)
{
+ trace_free_tunnel(tunnel);
sock_put(tunnel->sock);
/* the tunnel is freed in the socket destructor */
}
@@ -162,6 +164,8 @@ static void l2tp_session_free(struct l2tp_session *session)
{
struct l2tp_tunnel *tunnel = session->tunnel;
+ trace_free_session(session);
+
if (tunnel) {
if (WARN_ON(tunnel->magic != L2TP_TUNNEL_MAGIC))
goto out;
@@ -384,6 +388,8 @@ int l2tp_session_register(struct l2tp_session *session,
hlist_add_head(&session->hlist, head);
write_unlock_bh(&tunnel->hlist_lock);
+ trace_register_session(session);
+
return 0;
err_tlock_pnlock:
@@ -412,10 +418,6 @@ static void l2tp_recv_queue_skb(struct l2tp_session *session, struct sk_buff *sk
skb_queue_walk_safe(&session->reorder_q, skbp, tmp) {
if (L2TP_SKB_CB(skbp)->ns > ns) {
__skb_queue_before(&session->reorder_q, skbp, skb);
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: pkt %hu, inserted before %hu, reorder_q len=%d\n",
- session->name, ns, L2TP_SKB_CB(skbp)->ns,
- skb_queue_len(&session->reorder_q));
atomic_long_inc(&session->stats.rx_oos_packets);
goto out;
}
@@ -448,9 +450,7 @@ static void l2tp_recv_dequeue_skb(struct l2tp_session *session, struct sk_buff *
/* Bump our Nr */
session->nr++;
session->nr &= session->nr_max;
-
- l2tp_dbg(session, L2TP_MSG_SEQ, "%s: updated nr to %hu\n",
- session->name, session->nr);
+ trace_session_seqnum_update(session);
}
/* call private receive handler */
@@ -475,37 +475,27 @@ static void l2tp_recv_dequeue(struct l2tp_session *session)
start:
spin_lock_bh(&session->reorder_q.lock);
skb_queue_walk_safe(&session->reorder_q, skb, tmp) {
- if (time_after(jiffies, L2TP_SKB_CB(skb)->expires)) {
+ struct l2tp_skb_cb *cb = L2TP_SKB_CB(skb);
+
+ /* If the packet has been pending on the queue for too long, discard it */
+ if (time_after(jiffies, cb->expires)) {
atomic_long_inc(&session->stats.rx_seq_discards);
atomic_long_inc(&session->stats.rx_errors);
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: oos pkt %u len %d discarded (too old), waiting for %u, reorder_q_len=%d\n",
- session->name, L2TP_SKB_CB(skb)->ns,
- L2TP_SKB_CB(skb)->length, session->nr,
- skb_queue_len(&session->reorder_q));
+ trace_session_pkt_expired(session, cb->ns);
session->reorder_skip = 1;
__skb_unlink(skb, &session->reorder_q);
kfree_skb(skb);
continue;
}
- if (L2TP_SKB_CB(skb)->has_seq) {
+ if (cb->has_seq) {
if (session->reorder_skip) {
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: advancing nr to next pkt: %u -> %u",
- session->name, session->nr,
- L2TP_SKB_CB(skb)->ns);
session->reorder_skip = 0;
- session->nr = L2TP_SKB_CB(skb)->ns;
+ session->nr = cb->ns;
+ trace_session_seqnum_reset(session);
}
- if (L2TP_SKB_CB(skb)->ns != session->nr) {
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: holding oos pkt %u len %d, waiting for %u, reorder_q_len=%d\n",
- session->name, L2TP_SKB_CB(skb)->ns,
- L2TP_SKB_CB(skb)->length, session->nr,
- skb_queue_len(&session->reorder_q));
+ if (cb->ns != session->nr)
goto out;
- }
}
__skb_unlink(skb, &session->reorder_q);
@@ -538,14 +528,13 @@ static int l2tp_seq_check_rx_window(struct l2tp_session *session, u32 nr)
*/
static int l2tp_recv_data_seq(struct l2tp_session *session, struct sk_buff *skb)
{
- if (!l2tp_seq_check_rx_window(session, L2TP_SKB_CB(skb)->ns)) {
+ struct l2tp_skb_cb *cb = L2TP_SKB_CB(skb);
+
+ if (!l2tp_seq_check_rx_window(session, cb->ns)) {
/* Packet sequence number is outside allowed window.
* Discard it.
*/
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: pkt %u len %d discarded, outside window, nr=%u\n",
- session->name, L2TP_SKB_CB(skb)->ns,
- L2TP_SKB_CB(skb)->length, session->nr);
+ trace_session_pkt_outside_rx_window(session, cb->ns);
goto discard;
}
@@ -562,10 +551,10 @@ static int l2tp_recv_data_seq(struct l2tp_session *session, struct sk_buff *skb)
* is seen. After nr_oos_count_max in-sequence packets, reset the
* sequence number to re-enable packet reception.
*/
- if (L2TP_SKB_CB(skb)->ns == session->nr) {
+ if (cb->ns == session->nr) {
skb_queue_tail(&session->reorder_q, skb);
} else {
- u32 nr_oos = L2TP_SKB_CB(skb)->ns;
+ u32 nr_oos = cb->ns;
u32 nr_next = (session->nr_oos + 1) & session->nr_max;
if (nr_oos == nr_next)
@@ -576,17 +565,10 @@ static int l2tp_recv_data_seq(struct l2tp_session *session, struct sk_buff *skb)
session->nr_oos = nr_oos;
if (session->nr_oos_count > session->nr_oos_count_max) {
session->reorder_skip = 1;
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: %d oos packets received. Resetting sequence numbers\n",
- session->name, session->nr_oos_count);
}
if (!session->reorder_skip) {
atomic_long_inc(&session->stats.rx_seq_discards);
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: oos pkt %u len %d discarded, waiting for %u, reorder_q_len=%d\n",
- session->name, L2TP_SKB_CB(skb)->ns,
- L2TP_SKB_CB(skb)->length, session->nr,
- skb_queue_len(&session->reorder_q));
+ trace_session_pkt_oos(session, cb->ns);
goto discard;
}
skb_queue_tail(&session->reorder_q, skb);
@@ -716,9 +698,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
* configure it so.
*/
if (!session->lns_mode && !session->send_seq) {
- l2tp_info(session, L2TP_MSG_SEQ,
- "%s: requested to enable seq numbers by LNS\n",
- session->name);
+ trace_session_seqnum_lns_enable(session);
session->send_seq = 1;
l2tp_session_set_header_len(session, tunnel->version);
}
@@ -739,9 +719,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
* LAC is broken. Discard the frame.
*/
if (!session->lns_mode && session->send_seq) {
- l2tp_info(session, L2TP_MSG_SEQ,
- "%s: requested to disable seq numbers by LNS\n",
- session->name);
+ trace_session_seqnum_lns_disable(session);
session->send_seq = 0;
l2tp_session_set_header_len(session, tunnel->version);
} else if (session->send_seq) {
@@ -965,8 +943,7 @@ static int l2tp_build_l2tpv2_header(struct l2tp_session *session, void *buf)
*bufp++ = 0;
session->ns++;
session->ns &= 0xffff;
- l2tp_dbg(session, L2TP_MSG_SEQ, "%s: updated ns to %u\n",
- session->name, session->ns);
+ trace_session_seqnum_update(session);
}
return bufp - optr;
@@ -1002,9 +979,7 @@ static int l2tp_build_l2tpv3_header(struct l2tp_session *session, void *buf)
l2h = 0x40000000 | session->ns;
session->ns++;
session->ns &= 0xffffff;
- l2tp_dbg(session, L2TP_MSG_SEQ,
- "%s: updated ns to %u\n",
- session->name, session->ns);
+ trace_session_seqnum_update(session);
}
*((__be32 *)bufp) = htonl(l2h);
@@ -1544,6 +1519,8 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,
"l2tp_sock");
sk->sk_allocation = GFP_ATOMIC;
+ trace_register_tunnel(tunnel);
+
if (tunnel->fd >= 0)
sockfd_put(sock);
@@ -1564,6 +1541,7 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_register);
void l2tp_tunnel_delete(struct l2tp_tunnel *tunnel)
{
if (!test_and_set_bit(0, &tunnel->dead)) {
+ trace_delete_tunnel(tunnel);
l2tp_tunnel_inc_refcount(tunnel);
queue_work(l2tp_wq, &tunnel->del_work);
}
@@ -1575,6 +1553,7 @@ void l2tp_session_delete(struct l2tp_session *session)
if (test_and_set_bit(0, &session->dead))
return;
+ trace_delete_session(session);
l2tp_session_unhash(session);
l2tp_session_queue_purge(session);
if (session->session_close)
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 7/9] l2tp: remove custom logging macros
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
` (5 preceding siblings ...)
2020-08-21 10:47 ` [PATCH 6/9] l2tp: add tracepoints to l2tp_core.c Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 10:47 ` [PATCH 8/9] l2tp: remove tunnel and session debug flags field Tom Parkin
` (2 subsequent siblings)
9 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
All l2tp's informational and warning logging is now carried out using
standard kernel APIs.
Debugging information is now handled using tracepoints.
Now that no code is using the custom logging macros, remove them from
l2tp_core.h.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
net/l2tp/l2tp_core.h | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 835c4645a0ec..7a06ac135a9b 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -339,19 +339,6 @@ static inline int l2tp_v3_ensure_opt_in_linear(struct l2tp_session *session, str
return 0;
}
-#define l2tp_printk(ptr, type, func, fmt, ...) \
-do { \
- if (((ptr)->debug) & (type)) \
- func(fmt, ##__VA_ARGS__); \
-} while (0)
-
-#define l2tp_warn(ptr, type, fmt, ...) \
- l2tp_printk(ptr, type, pr_warn, fmt, ##__VA_ARGS__)
-#define l2tp_info(ptr, type, fmt, ...) \
- l2tp_printk(ptr, type, pr_info, fmt, ##__VA_ARGS__)
-#define l2tp_dbg(ptr, type, fmt, ...) \
- l2tp_printk(ptr, type, pr_debug, fmt, ##__VA_ARGS__)
-
#define MODULE_ALIAS_L2TP_PWTYPE(type) \
MODULE_ALIAS("net-l2tp-type-" __stringify(type))
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 8/9] l2tp: remove tunnel and session debug flags field
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
` (6 preceding siblings ...)
2020-08-21 10:47 ` [PATCH 7/9] l2tp: remove custom logging macros Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 10:47 ` [PATCH 9/9] docs: networking: add tracepoint info to l2tp.rst Tom Parkin
2020-08-21 11:25 ` [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints James Chapman
9 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
The l2tp subsystem now uses standard kernel logging APIs for
informational and warning messages, and tracepoints for debug
information.
Now that the tunnel and session debug flags are unused, remove the field
from the core structures.
Various system calls (in the case of l2tp_ppp) and netlink messages
handle the getting and setting of debug flags. To avoid userspace
breakage don't modify the API of these calls; simply ignore set
requests, and send dummy data for get requests.
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
include/uapi/linux/if_pppol2tp.h | 2 +-
include/uapi/linux/l2tp.h | 6 ++++--
net/l2tp/l2tp_core.c | 8 --------
net/l2tp/l2tp_core.h | 4 ----
net/l2tp/l2tp_debugfs.c | 4 ++--
net/l2tp/l2tp_netlink.c | 16 ++--------------
net/l2tp/l2tp_ppp.c | 15 ++++++++-------
7 files changed, 17 insertions(+), 38 deletions(-)
diff --git a/include/uapi/linux/if_pppol2tp.h b/include/uapi/linux/if_pppol2tp.h
index 060b4d1f3129..a91044328bc9 100644
--- a/include/uapi/linux/if_pppol2tp.h
+++ b/include/uapi/linux/if_pppol2tp.h
@@ -75,7 +75,7 @@ struct pppol2tpv3in6_addr {
};
/* Socket options:
- * DEBUG - bitmask of debug message categories
+ * DEBUG - bitmask of debug message categories (not used)
* SENDSEQ - 0 => don't send packets with sequence numbers
* 1 => send packets with sequence numbers
* RECVSEQ - 0 => receive packet sequence numbers are optional
diff --git a/include/uapi/linux/l2tp.h b/include/uapi/linux/l2tp.h
index 61158f5a1a5b..88a0d32b8c07 100644
--- a/include/uapi/linux/l2tp.h
+++ b/include/uapi/linux/l2tp.h
@@ -108,7 +108,7 @@ enum {
L2TP_ATTR_VLAN_ID, /* u16 (not used) */
L2TP_ATTR_COOKIE, /* 0, 4 or 8 bytes */
L2TP_ATTR_PEER_COOKIE, /* 0, 4 or 8 bytes */
- L2TP_ATTR_DEBUG, /* u32, enum l2tp_debug_flags */
+ L2TP_ATTR_DEBUG, /* u32, enum l2tp_debug_flags (not used) */
L2TP_ATTR_RECV_SEQ, /* u8 */
L2TP_ATTR_SEND_SEQ, /* u8 */
L2TP_ATTR_LNS_MODE, /* u8 */
@@ -177,7 +177,9 @@ enum l2tp_seqmode {
};
/**
- * enum l2tp_debug_flags - debug message categories for L2TP tunnels/sessions
+ * enum l2tp_debug_flags - debug message categories for L2TP tunnels/sessions.
+ *
+ * Unused.
*
* @L2TP_MSG_DEBUG: verbose debug (if compiled in)
* @L2TP_MSG_CONTROL: userspace - kernel interface
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 651c08dc9bcf..e6e26123dee6 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1405,16 +1405,12 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
tunnel->version = version;
tunnel->tunnel_id = tunnel_id;
tunnel->peer_tunnel_id = peer_tunnel_id;
- tunnel->debug = L2TP_DEFAULT_DEBUG_FLAGS;
tunnel->magic = L2TP_TUNNEL_MAGIC;
sprintf(&tunnel->name[0], "tunl %u", tunnel_id);
rwlock_init(&tunnel->hlist_lock);
tunnel->acpt_newsess = true;
- if (cfg)
- tunnel->debug = cfg->debug;
-
tunnel->encap = encap;
refcount_set(&tunnel->ref_count, 1);
@@ -1612,12 +1608,8 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
INIT_HLIST_NODE(&session->hlist);
INIT_HLIST_NODE(&session->global_hlist);
- /* Inherit debug options from tunnel */
- session->debug = tunnel->debug;
-
if (cfg) {
session->pwtype = cfg->pw_type;
- session->debug = cfg->debug;
session->send_seq = cfg->send_seq;
session->recv_seq = cfg->recv_seq;
session->lns_mode = cfg->lns_mode;
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 7a06ac135a9b..07249c5f22ef 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -51,7 +51,6 @@ struct l2tp_session_cfg {
unsigned int lns_mode:1; /* behave as LNS?
* LAC enables sequence numbers under LNS control.
*/
- int debug; /* bitmask of debug message categories */
u16 l2specific_type; /* Layer 2 specific type */
u8 cookie[8]; /* optional cookie */
int cookie_len; /* 0, 4 or 8 bytes */
@@ -98,7 +97,6 @@ struct l2tp_session {
unsigned int lns_mode:1; /* behave as LNS?
* LAC enables sequence numbers under LNS control.
*/
- int debug; /* bitmask of debug message categories */
int reorder_timeout; /* configured reorder timeout (in jiffies) */
int reorder_skip; /* set if skip to next nr */
enum l2tp_pwtype pwtype;
@@ -132,7 +130,6 @@ struct l2tp_session {
/* L2TP tunnel configuration */
struct l2tp_tunnel_cfg {
- int debug; /* bitmask of debug message categories */
enum l2tp_encap_type encap;
/* Used only for kernel-created sockets */
@@ -173,7 +170,6 @@ struct l2tp_tunnel {
int version; /* 2=>L2TPv2, 3=>L2TPv3 */
char name[L2TP_TUNNEL_NAME_MAX]; /* for logging */
- int debug; /* bitmask of debug message categories */
enum l2tp_encap_type encap;
struct l2tp_stats stats;
diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
index 96cb9601c21b..bca75bef8282 100644
--- a/net/l2tp/l2tp_debugfs.c
+++ b/net/l2tp/l2tp_debugfs.c
@@ -167,7 +167,7 @@ static void l2tp_dfs_seq_tunnel_show(struct seq_file *m, void *v)
tunnel->sock ? refcount_read(&tunnel->sock->sk_refcnt) : 0,
refcount_read(&tunnel->ref_count));
seq_printf(m, " %08x rx %ld/%ld/%ld rx %ld/%ld/%ld\n",
- tunnel->debug,
+ 0,
atomic_long_read(&tunnel->stats.tx_packets),
atomic_long_read(&tunnel->stats.tx_bytes),
atomic_long_read(&tunnel->stats.tx_errors),
@@ -192,7 +192,7 @@ static void l2tp_dfs_seq_session_show(struct seq_file *m, void *v)
session->recv_seq ? 'R' : '-',
session->send_seq ? 'S' : '-',
session->lns_mode ? "LNS" : "LAC",
- session->debug,
+ 0,
jiffies_to_msecs(session->reorder_timeout));
seq_printf(m, " offset 0 l2specific %hu/%hu\n",
session->l2specific_type, l2tp_get_l2specific_len(session));
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index def78eebca4c..31a1e27eab20 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -229,9 +229,6 @@ static int l2tp_nl_cmd_tunnel_create(struct sk_buff *skb, struct genl_info *info
goto out;
}
- if (attrs[L2TP_ATTR_DEBUG])
- cfg.debug = nla_get_u32(attrs[L2TP_ATTR_DEBUG]);
-
ret = -EINVAL;
switch (cfg.encap) {
case L2TP_ENCAPTYPE_UDP:
@@ -307,9 +304,6 @@ static int l2tp_nl_cmd_tunnel_modify(struct sk_buff *skb, struct genl_info *info
goto out;
}
- if (info->attrs[L2TP_ATTR_DEBUG])
- tunnel->debug = nla_get_u32(info->attrs[L2TP_ATTR_DEBUG]);
-
ret = l2tp_tunnel_notify(&l2tp_nl_family, info,
tunnel, L2TP_CMD_TUNNEL_MODIFY);
@@ -400,7 +394,7 @@ static int l2tp_nl_tunnel_send(struct sk_buff *skb, u32 portid, u32 seq, int fla
if (nla_put_u8(skb, L2TP_ATTR_PROTO_VERSION, tunnel->version) ||
nla_put_u32(skb, L2TP_ATTR_CONN_ID, tunnel->tunnel_id) ||
nla_put_u32(skb, L2TP_ATTR_PEER_CONN_ID, tunnel->peer_tunnel_id) ||
- nla_put_u32(skb, L2TP_ATTR_DEBUG, tunnel->debug) ||
+ nla_put_u32(skb, L2TP_ATTR_DEBUG, 0) ||
nla_put_u16(skb, L2TP_ATTR_ENCAP_TYPE, tunnel->encap))
goto nla_put_failure;
@@ -605,9 +599,6 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
cfg.ifname = nla_data(info->attrs[L2TP_ATTR_IFNAME]);
}
- if (info->attrs[L2TP_ATTR_DEBUG])
- cfg.debug = nla_get_u32(info->attrs[L2TP_ATTR_DEBUG]);
-
if (info->attrs[L2TP_ATTR_RECV_SEQ])
cfg.recv_seq = nla_get_u8(info->attrs[L2TP_ATTR_RECV_SEQ]);
@@ -689,9 +680,6 @@ static int l2tp_nl_cmd_session_modify(struct sk_buff *skb, struct genl_info *inf
goto out;
}
- if (info->attrs[L2TP_ATTR_DEBUG])
- session->debug = nla_get_u32(info->attrs[L2TP_ATTR_DEBUG]);
-
if (info->attrs[L2TP_ATTR_RECV_SEQ])
session->recv_seq = nla_get_u8(info->attrs[L2TP_ATTR_RECV_SEQ]);
@@ -730,7 +718,7 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
nla_put_u32(skb, L2TP_ATTR_SESSION_ID, session->session_id) ||
nla_put_u32(skb, L2TP_ATTR_PEER_CONN_ID, tunnel->peer_tunnel_id) ||
nla_put_u32(skb, L2TP_ATTR_PEER_SESSION_ID, session->peer_session_id) ||
- nla_put_u32(skb, L2TP_ATTR_DEBUG, session->debug) ||
+ nla_put_u32(skb, L2TP_ATTR_DEBUG, 0) ||
nla_put_u16(skb, L2TP_ATTR_PW_TYPE, session->pwtype))
goto nla_put_failure;
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index bd6bb17dfadb..450637ffa557 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -702,7 +702,6 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
if (!tunnel) {
struct l2tp_tunnel_cfg tcfg = {
.encap = L2TP_ENCAPTYPE_UDP,
- .debug = 0,
};
/* Prevent l2tp_tunnel_register() from trying to set up
@@ -1147,7 +1146,7 @@ static int pppol2tp_tunnel_setsockopt(struct sock *sk,
switch (optname) {
case PPPOL2TP_SO_DEBUG:
- tunnel->debug = val;
+ /* Tunnel debug flags option is deprecated */
break;
default:
@@ -1199,7 +1198,7 @@ static int pppol2tp_session_setsockopt(struct sock *sk,
break;
case PPPOL2TP_SO_DEBUG:
- session->debug = val;
+ /* Session debug flags option is deprecated */
break;
case PPPOL2TP_SO_REORDERTO:
@@ -1271,7 +1270,8 @@ static int pppol2tp_tunnel_getsockopt(struct sock *sk,
switch (optname) {
case PPPOL2TP_SO_DEBUG:
- *val = tunnel->debug;
+ /* Tunnel debug flags option is deprecated */
+ *val = 0;
break;
default:
@@ -1304,7 +1304,8 @@ static int pppol2tp_session_getsockopt(struct sock *sk,
break;
case PPPOL2TP_SO_DEBUG:
- *val = session->debug;
+ /* Session debug flags option is deprecated */
+ *val = 0;
break;
case PPPOL2TP_SO_REORDERTO:
@@ -1496,7 +1497,7 @@ static void pppol2tp_seq_tunnel_show(struct seq_file *m, void *v)
(tunnel == tunnel->sock->sk_user_data) ? 'Y' : 'N',
refcount_read(&tunnel->ref_count) - 1);
seq_printf(m, " %08x %ld/%ld/%ld %ld/%ld/%ld\n",
- tunnel->debug,
+ 0,
atomic_long_read(&tunnel->stats.tx_packets),
atomic_long_read(&tunnel->stats.tx_bytes),
atomic_long_read(&tunnel->stats.tx_errors),
@@ -1542,7 +1543,7 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
session->recv_seq ? 'R' : '-',
session->send_seq ? 'S' : '-',
session->lns_mode ? "LNS" : "LAC",
- session->debug,
+ 0,
jiffies_to_msecs(session->reorder_timeout));
seq_printf(m, " %hu/%hu %ld/%ld/%ld %ld/%ld/%ld\n",
session->nr, session->ns,
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 9/9] docs: networking: add tracepoint info to l2tp.rst
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
` (7 preceding siblings ...)
2020-08-21 10:47 ` [PATCH 8/9] l2tp: remove tunnel and session debug flags field Tom Parkin
@ 2020-08-21 10:47 ` Tom Parkin
2020-08-21 11:25 ` [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints James Chapman
9 siblings, 0 replies; 13+ messages in thread
From: Tom Parkin @ 2020-08-21 10:47 UTC (permalink / raw)
To: netdev; +Cc: jchapman, Tom Parkin
Update l2tp.rst to:
* remove information about the obsolete debug flags and their use
* include information about tracepoints for l2tp
Signed-off-by: Tom Parkin <tparkin@katalix.com>
---
Documentation/networking/l2tp.rst | 37 ++++++++++++-------------------
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/Documentation/networking/l2tp.rst b/Documentation/networking/l2tp.rst
index 693ea0e3ec26..498b382d25a0 100644
--- a/Documentation/networking/l2tp.rst
+++ b/Documentation/networking/l2tp.rst
@@ -455,31 +455,16 @@ l2tp help`` for more information.
Debugging
---------
-The L2TP subsystem offers a debug scheme where kernel trace messages
-may be optionally enabled per tunnel and per session. Care is needed
-when debugging a live system since the messages are not rate-limited
-and a busy system could be swamped. Userspace uses setsockopt on the
-PPPoX socket to set a debug mask, or ``L2TP_ATTR_DEBUG`` in netlink
-Create and Modify commands.
+The L2TP subsystem offers a range of debugging interfaces through the
+debugfs filesystem.
-The following debug mask bits are defined:-
-
-================ ==============================
-L2TP_MSG_DEBUG verbose debug (if compiled in)
-L2TP_MSG_CONTROL userspace - kernel interface
-L2TP_MSG_SEQ sequence numbers handling
-L2TP_MSG_DATA data packets
-================ ==============================
-
-Sessions inherit default debug flags from the parent tunnel.
-
-If enabled, files under a l2tp debugfs directory can be used to dump
-kernel state about L2TP tunnels and sessions. To access it, the
-debugfs filesystem must first be mounted::
+To access these interfaces, the debugfs filesystem must first be mounted::
# mount -t debugfs debugfs /debug
-Files under the l2tp directory can then be accessed::
+Files under the l2tp directory can then be accessed, providing a summary
+of the current population of tunnel and session contexts existing in the
+kernel::
# cat /debug/l2tp/tunnels
@@ -488,8 +473,14 @@ state information because the file format is subject to change. It is
implemented to provide extra debug information to help diagnose
problems. Applications should instead use the netlink API.
-/proc/net/pppol2tp is also provided for backwards compatibility with
-the original pppol2tp code. It lists information about L2TPv2
+In addition the L2TP subsystem implements tracepoints using the standard
+kernel event tracing API. The available L2TP events can be reviewed as
+follows::
+
+ # find /debug/tracing/events/l2tp
+
+Finally, /proc/net/pppol2tp is also provided for backwards compatibility
+with the original pppol2tp code. It lists information about L2TPv2
tunnels and sessions only. Its use is discouraged.
Internal Implementation
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints
2020-08-21 10:47 [PATCH net-next 0/9] l2tp: replace custom logging code with tracepoints Tom Parkin
` (8 preceding siblings ...)
2020-08-21 10:47 ` [PATCH 9/9] docs: networking: add tracepoint info to l2tp.rst Tom Parkin
@ 2020-08-21 11:25 ` James Chapman
9 siblings, 0 replies; 13+ messages in thread
From: James Chapman @ 2020-08-21 11:25 UTC (permalink / raw)
To: Tom Parkin, netdev
On 21/08/2020 11:47, Tom Parkin wrote:
> The l2tp subsystem implemented custom logging macros for debugging
> purposes which were controlled using a set of debugging flags in each
> tunnel and session structure.
>
> A more standard and easier-to-use approach is to use tracepoints.
>
> This patchset refactors l2tp to:
>
> * remove excessive logging
> * tweak useful log messages to use the standard pr_* calls for logging
> rather than the l2tp wrappers
> * replace debug-level logging with tracepoints
> * add tracepoints for capturing tunnel and session lifetime events
Acked-by: James Chapman <jchapman@katalix.com>
^ permalink raw reply [flat|nested] 13+ messages in thread