31 Jul, 2020
6 commits
-
* Improve the description of the key l2tp subsystem data structures.
* Add high-level description of the main APIs for interacting with l2tp
core.
* Add documentation for the l2tp netlink session command callbacks.
* Document the session pseudowire callbacks.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
All of the l2tp subsystem's exported symbols are exported using
EXPORT_SYMBOL_GPL, except for l2tp_recv_common and l2tp_ioctl.These functions alone are not useful without the rest of the l2tp
infrastructure, so there's no practical benefit to these symbols using a
different export policy.Change these exports to use EXPORT_SYMBOL_GPL for consistency with the
rest of l2tp.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
The structure of an L2TP data packet header varies depending on the
version of the L2TP protocol being used.struct l2tp_session used to have a build_header callback to abstract
this difference away. It's clearer to simply choose the correct
function to use when building the data packet (and we save on the
function pointer in the session structure).This approach does mean dereferencing the parent tunnel structure in
order to determine the tunnel version, but we're doing that in the
transmit path in any case.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
l2tp_session_delete is used to schedule a session instance for deletion.
The function itself always returns zero, and none of its direct callers
check its return value, so have the function return void.This change de-facto changes the l2tp netlink session_delete callback
prototype since all pseudowires currently use l2tp_session_delete for
their implementation of that operation.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
Tunnel and session instances are reference counted, and shouldn't be
directly freed by pseudowire code.Rather than exporting l2tp_tunnel_free and l2tp_session_free, make them
private to l2tp_core.c, and export the refcount functions instead.In order to do this, the refcount functions cannot be declared as
inline. Since the codepaths which take and drop tunnel and session
references are not directly in the datapath this shouldn't cause
performance issues.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
When __l2tp_session_unhash was first added it was used outside of
l2tp_core.c, but that's no longer the case.As such, there's no longer a need to export the function. Make it
private inside l2tp_core.c, and relocate it to avoid having to declare
the function prototype in l2tp_core.h.Since the function is no longer used outside l2tp_core.c, remove the
"__" prefix since we don't need to indicate anything special about its
expected use to callers.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller
25 Jul, 2020
10 commits
-
l2tp_session_free called BUG_ON if the tunnel magic feather value wasn't
correct. The intent of this was to catch lifetime bugs; for example
early tunnel free due to incorrect use of reference counts.Since the tunnel magic feather being wrong indicates either early free
or structure corruption, we can avoid doing more damage by simply
leaving the tunnel structure alone. If the tunnel refcount isn't
dropped when it should be, the tunnel instance will remain in the
kernel, resulting in the tunnel structure and socket leaking.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
l2tp_session_free is only called by l2tp_session_dec_refcount when the
reference count reaches zero, so it's of limited value to validate the
reference count value in l2tp_session_free itself.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
l2tp_session_queue_purge is used during session shutdown to drop any
skbs queued for reordering purposes according to L2TP dataplane rules.The BUG_ON in this function checks the session magic feather in an
attempt to catch lifetime bugs.Rather than crashing the kernel with a BUG_ON, we can simply WARN_ON and
refuse to do anything more -- in the worst case this could result in a
leak. However this is highly unlikely given that the session purge only
occurs from codepaths which have obtained the session by means of a lookup
via. the parent tunnel and which check the session "dead" flag to
protect against shutdown races.While we're here, have l2tp_session_queue_purge return void rather than
an integer, since neither of the callsites checked the return value.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
checkpatch advises that WARN_ON and recovery code are preferred over
BUG_ON which crashes the kernel.l2tp_ppp has a BUG_ON check of struct seq_file's private pointer in
pppol2tp_seq_start prior to accessing data through that pointer.Rather than crashing, we can simply bail out early and return NULL in
order to terminate the seq file processing in much the same way as we do
when reaching the end of tunnel/session instances to render.Retain a WARN_ON to help trace possible bugs in this area.
Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
checkpatch advises that WARN_ON and recovery code are preferred over
BUG_ON which crashes the kernel.l2tp_ppp.c's BUG_ON checks of the l2tp session structure's "magic" field
occur in code paths where it's reasonably easy to recover:* In the case of pppol2tp_sock_to_session, we can return NULL and the
caller will bail out appropriately. There is no change required to
any of the callsites of this function since they already handle
pppol2tp_sock_to_session returning NULL.* In the case of pppol2tp_session_destruct we can just avoid
decrementing the reference count on the suspect session structure.
In the worst case scenario this results in a memory leak, which is
preferable to a crash.Convert these uses of BUG_ON to WARN_ON accordingly.
Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
l2tp_tunnel_closeall is only called from l2tp_core.c, and it's easy
to statically analyse the code path calling it to validate that it
should never be passed a NULL tunnel pointer.Having a BUG_ON checking the tunnel pointer triggers a checkpatch
warning. Since the BUG_ON is of no value, remove it to avoid the
warning.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
l2tp_session_queue_purge is only called from l2tp_core.c, and it's easy
to statically analyse the code paths calling it to validate that it
should never be passed a NULL session pointer.Having a BUG_ON checking the session pointer triggers a checkpatch
warning. Since the BUG_ON is of no value, remove it to avoid the
warning.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
l2tp_dfs_seq_start had a BUG_ON to catch a possible programming error in
l2tp_dfs_seq_open.Since we can easily bail out of l2tp_dfs_seq_start, prefer to do that
and flag the error with a WARN_ON rather than crashing the kernel.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
checkpatch warns about multiple assignments.
Update l2tp accordingly.
Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
Rework the remaining setsockopt code to pass a sockptr_t instead of a
plain user pointer. This removes the last remaining set_fs(KERNEL_DS)
outside of architecture specific code.Signed-off-by: Christoph Hellwig
Acked-by: Stefan Schmidt [ieee802154]
Acked-by: Matthieu Baerts
Signed-off-by: David S. Miller
24 Jul, 2020
6 commits
-
Passing "sizeof(struct blah)" in kzalloc calls is less readable,
potentially prone to future bugs if the type of the pointer is changed,
and triggers checkpatch warnings.Tweak the kzalloc calls in l2tp which use this form to avoid the
warning.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
When creating an L2TP tunnel using the netlink API, userspace must
either pass a socket FD for the tunnel to use (for managed tunnels),
or specify the tunnel source/destination address (for unmanaged
tunnels).Since source/destination addresses may be AF_INET or AF_INET6, the l2tp
netlink code has conditionally compiled blocks to support IPv6.Rather than embedding these directly into l2tp_nl_cmd_tunnel_create
(where it makes the code difficult to read and confuses checkpatch to
boot) split the handling of address-related attributes into a separate
function.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
l2tp_nl_tunnel_send has conditionally compiled code to support AF_INET6,
which makes the code difficult to follow and triggers checkpatch
warnings.Split the code out into functions to handle the AF_INET v.s. AF_INET6
cases, which both improves readability and resolves the checkpatch
warnings.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
checkpatch warns about indentation and brace balancing around the
conditionally compiled code for AF_INET6 support in
l2tp_dfs_seq_tunnel_show.By adding another check on the socket address type we can make the code
more readable while removing the checkpatch warning.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
These checks are all simple and don't benefit from extra braces to
clarify intent. Remove them for easier-reading code.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
checkpatch warns about comparisons to NULL, e.g.
CHECK: Comparison to NULL could be written "!rt"
#474: FILE: net/l2tp/l2tp_ip.c:474:
+ if (rt == NULL) {These sort of comparisons are generally clearer and more readable
the way checkpatch suggests, so update l2tp accordingly.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller
23 Jul, 2020
10 commits
-
checkpatch warned about the L2TP_SKB_CB macro's use of its argument: add
braces to avoid the problem.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
In l2tp_core.c both l2tp_tunnel_create and l2tp_session_create take
quite a number of arguments and have a correspondingly long prototype.This is both quite difficult to scan visually, and triggers checkpatch
warnings.Add a line break to make these function prototypes more readable.
Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
checkpatch warns about use of seq_printf where seq_puts would do.
Modify l2tp_debugfs accordingly.
Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
Use BIT(x) rather than (1<
Signed-off-by: David S. Miller -
Reported by checkpatch:
"WARNING: function definition argument 'struct sock *'
should also have an identifier name"Add an identifier name to help document the prototype.
Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
l2tp_core has conditionally compiled code in l2tp_xmit_skb for IPv6
support. The structure of this code triggered a checkpatch warning
due to incorrect indentation.Fix up the indentation to address the checkpatch warning.
Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
Arguments should be aligned with the function call open parenthesis as
per checkpatch. Tweak some function calls which were not aligned
correctly.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
Some l2tp code had line breaks which made the code more difficult to
read. These were originally motivated by the 80-character line width
coding guidelines, but were actually a negative from the perspective of
trying to follow the code.Remove these linebreaks for clearer code, even if we do exceed 80
characters in width in some places.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
Modify some l2tp comments to better adhere to kernel coding style, as
reported by checkpatch.pl.Add descriptive comments for the l2tp per-net spinlocks to document
their use.Fix an incorrect comment in l2tp_recv_common:
RFC2661 section 5.4 states that:
"The LNS controls enabling and disabling of sequence numbers by sending a
data message with or without sequence numbers present at any time during
the life of a session."l2tp handles this correctly in l2tp_recv_common, but the comment around
the code was incorrect and confusing. Fix up the comment accordingly.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller -
Fix up various whitespace issues as reported by checkpatch.pl:
* remove spaces around operators where appropriate,
* add missing blank lines following declarations,
* remove multiple blank lines, or trailing blank lines at the end of
functions.Signed-off-by: Tom Parkin
Signed-off-by: David S. Miller
20 Jul, 2020
3 commits
-
Handle the few cases that need special treatment in-line using
in_compat_syscall(). This also removes all the now unused
compat_{get,set}sockopt methods.Signed-off-by: Christoph Hellwig
Signed-off-by: David S. Miller -
Handle the few cases that need special treatment in-line using
in_compat_syscall().Signed-off-by: Christoph Hellwig
Signed-off-by: David S. Miller -
Add the compat handling to sock_common_{get,set}sockopt instead,
keyed of in_compat_syscall(). This allow to remove the now unused
->compat_{get,set}sockopt methods from struct proto_ops.Signed-off-by: Christoph Hellwig
Acked-by: Matthieu Baerts
Acked-by: Stefan Schmidt
Signed-off-by: David S. Miller
11 Jul, 2020
1 commit
-
All conflicts seemed rather trivial, with some guidance from
Saeed Mameed on the tc_ct.c one.Signed-off-by: David S. Miller
09 Jul, 2020
1 commit
-
In the tx path of l2tp, l2tp_xmit_skb() calls skb_dst_set() to set
skb's dst. However, it will eventually call inet6_csk_xmit() or
ip_queue_xmit() where skb's dst will be overwritten by:skb_dst_set_noref(skb, dst);
without releasing the old dst in skb. Then it causes dst/dev refcnt leak:
unregister_netdevice: waiting for eth0 to become free. Usage count = 1
This can be reproduced by simply running:
# modprobe l2tp_eth && modprobe l2tp_ip
# sh ./tools/testing/selftests/net/l2tp.shSo before going to inet6_csk_xmit() or ip_queue_xmit(), skb's dst
should be dropped. This patch is to fix it by removing skb_dst_set()
from l2tp_xmit_skb() and moving skb_dst_drop() into l2tp_xmit_core().Fixes: 3557baabf280 ("[L2TP]: PPP over L2TP driver core")
Reported-by: Hangbin Liu
Signed-off-by: Xin Long
Acked-by: James Chapman
Tested-by: James Chapman
Signed-off-by: David S. Miller
29 Jun, 2020
1 commit
-
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, but the implementation in this
driver returns an 'int'.Fix this by returning 'netdev_tx_t' in this driver too.
Signed-off-by: Luc Van Oostenryck
Signed-off-by: David S. Miller
14 Jun, 2020
1 commit
-
Since commit 84af7a6194e4 ("checkpatch: kconfig: prefer 'help' over
'---help---'"), the number of '---help---' has been gradually
decreasing, but there are still more than 2400 instances.This commit finishes the conversion. While I touched the lines,
I also fixed the indentation.There are a variety of indentation styles found.
a) 4 spaces + '---help---'
b) 7 spaces + '---help---'
c) 8 spaces + '---help---'
d) 1 space + 1 tab + '---help---'
e) 1 tab + '---help---' (correct indentation)
f) 1 tab + 1 space + '---help---'
g) 1 tab + 2 spaces + '---help---'In order to convert all of them to 1 tab + 'help', I ran the
following commend:$ find . -name 'Kconfig*' | xargs sed -i 's/^[[:space:]]*---help---/\thelp/'
Signed-off-by: Masahiro Yamada
01 Jun, 2020
1 commit
-
xdp_umem.c had overlapping changes between the 64-bit math fix
for the calculation of npgs and the removal of the zerocopy
memory type which got rid of the chunk_size_nohdr member.The mlx5 Kconfig conflict is a case where we just take the
net-next copy of the Kconfig entry dependency as it takes on
the ESWITCH dependency by one level of indirection which is
what the 'net' conflicting change is trying to ensure.Signed-off-by: David S. Miller