* Re: [PATCH v4 net-next] net: mvneta: implement .set_wol and .get_wol
From: Andrew Lunn @ 2017-01-23 18:10 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: thomas.petazzoni, davem, netdev, linux-kernel, Jingju Hou
In-Reply-To: <20170123065507.2557-1-jszhang@marvell.com>
On Mon, Jan 23, 2017 at 02:55:07PM +0800, Jisheng Zhang wrote:
> From: Jingju Hou <houjingj@marvell.com>
>
> From: Jingju Hou <houjingj@marvell.com>
>
> The mvneta itself does not support WOL, but the PHY might.
> So pass the calls to the PHY
>
> Signed-off-by: Jingju Hou <houjingj@marvell.com>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
> since v3:
> - really fix the build error
Keep trying....
But maybe tomorrow, after you have taken the pause Dave said you
should take, and maybe ask Jingju to really review it, in detail.
>
> since v2,v1:
> - using phy_dev member in struct net_device
> - add commit msg
>
> drivers/net/ethernet/marvell/mvneta.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 6dcc951af0ff..02611fa1c3b8 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -3929,6 +3929,25 @@ static int mvneta_ethtool_get_rxfh(struct net_device *dev, u32 *indir, u8 *key,
> return 0;
> }
>
> +static void mvneta_ethtool_get_wol(struct net_device *dev,
> + struct ethtool_wolinfo *wol)
> +{
> + wol->supported = 0;
> + wol->wolopts = 0;
> +
> + if (dev->phydev)
> + return phy_ethtool_get_wol(dev->phydev, wol);
This is a void function. And you are returning a value. And
phy_ethtool_get_wol() is also a void function, so does not actually
return anything.
0-day was telling you this. Please read what is said.
Andrew
^ permalink raw reply
* Re: [net-next 0/8] More MV88E6390 patches
From: David Miller @ 2017-01-23 18:06 UTC (permalink / raw)
To: andrew; +Cc: netdev, vivien.didelot
In-Reply-To: <1484955062-26718-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Sat, 21 Jan 2017 00:30:54 +0100
> This is the ongoing work to add support for the Marvell 9390 family of
> switches. There are now two MDIO busses, one for the internal PHYs and
> an external bus for external PHYs. Add support for this external bus.
I am anticipating another respin of this based upon feedback, just
FYI.
Correct me if my expectations are wrong.
Thanks.
^ permalink raw reply
* Re: wrong smp_mb__after_atomic() in tcp_check_space() ?
From: Eric Dumazet @ 2017-01-23 18:04 UTC (permalink / raw)
To: Jason Baron
Cc: Oleg Nesterov, David S. Miller, Herbert Xu, Yauheni Kaliuta,
netdev
In-Reply-To: <f5f9c542-2b23-9df9-098c-ad4007bf7e78@akamai.com>
On Mon, 2017-01-23 at 11:56 -0500, Jason Baron wrote:
> On 01/23/2017 09:30 AM, Oleg Nesterov wrote:
> > Hello,
> >
> > smp_mb__after_atomic() looks wrong and misleading, sock_reset_flag() does the
> > non-atomic __clear_bit() and thus it can not guarantee test_bit(SOCK_NOSPACE)
> > (non-atomic too) won't be reordered.
> >
>
> Indeed. Here's a bit of discussion on it:
> http://marc.info/?l=linux-netdev&m=146662325920596&w=2
>
> > It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
> > and the patch looks correct in that we need the barriers in tcp_check_space()
> > and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?
> >
>
> Yes, I think it should be upgraded to an smp_mb() there. If you agree
> with this analysis, I will send a patch to upgrade it. Note, I did not
> actually run into this race in practice.
SOCK_QUEUE_SHRUNK is used locally in TCP, it is not used by tcp_poll().
(Otherwise it would be using atomic set/clear operations)
I do not see obvious reason why we have this smp_mb__after_atomic() in
tcp_check_space().
But looking at this code, it seems we lack one barrier if sk_sndbuf is
ever increased. Fortunately this almost never happen during TCP session
lifetime...
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bfa165cc455ad0a9aea44964aa663dbe6085aebd..3692e9f4c852cebf8c4d46c141f112e75e4ae66d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -331,8 +331,13 @@ static void tcp_sndbuf_expand(struct sock *sk)
sndmem = ca_ops->sndbuf_expand ? ca_ops->sndbuf_expand(sk) : 2;
sndmem *= nr_segs * per_mss;
- if (sk->sk_sndbuf < sndmem)
+ if (sk->sk_sndbuf < sndmem) {
sk->sk_sndbuf = min(sndmem, sysctl_tcp_wmem[2]);
+ /* Paired with second sk_stream_is_writeable(sk)
+ * test from tcp_poll()
+ */
+ smp_wmb();
+ }
}
/* 2. Tuning advertised window (window_clamp, rcv_ssthresh)
^ permalink raw reply related
* Re: [PATCH] [net-next][v2] net: qcom/emac: claim the irq only when the device is opened
From: David Miller @ 2017-01-23 18:03 UTC (permalink / raw)
To: timur; +Cc: netdev, LinoSanfilippo
In-Reply-To: <1484954464-2622-2-git-send-email-timur@codeaurora.org>
From: Timur Tabi <timur@codeaurora.org>
Date: Fri, 20 Jan 2017 17:21:04 -0600
> During reset, functions emac_mac_down() and emac_mac_up() are called,
> so we don't want to free and claim the IRQ unnecessarily. Move those
> operations to open/close.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
> ---
>
> Notes:
> v2: keep synchronize_irq call where it is
Applied.
^ permalink raw reply
* Re: [PATCH] [net-next] net: qcom/emac: rename emac_phy to emac_sgmii and move it
From: David Miller @ 2017-01-23 17:55 UTC (permalink / raw)
To: timur; +Cc: netdev, LinoSanfilippo
In-Reply-To: <1484954464-2622-1-git-send-email-timur@codeaurora.org>
From: Timur Tabi <timur@codeaurora.org>
Date: Fri, 20 Jan 2017 17:21:03 -0600
> The EMAC has an internal PHY that is often called the "SGMII". This
> SGMII is also connected to an external PHY, which is managed by phylib.
> These dual PHYs often cause confusion. In this case, the data structure
> for managing the SGMII was mis-named and located in the wrong header file.
>
> Structure emac_phy is renamed to emac_sgmii to clearly indicate it applies
> to the internal PHY only. It also also moved from emac_phy.h (which
> supports the external PHY) to emac_sgmii.h (where it belongs).
>
> To keep the changes minimal, only the structure name is changed, not
> the names of any variables of that type.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: adaptec: starfire: add checks for dma mapping errors
From: David Miller @ 2017-01-23 17:53 UTC (permalink / raw)
To: khoroshilov; +Cc: ionut, netdev, linux-kernel, ldv-project
In-Reply-To: <1484952744-9815-1-git-send-email-khoroshilov@ispras.ru>
From: Alexey Khoroshilov <khoroshilov@ispras.ru>
Date: Sat, 21 Jan 2017 01:52:24 +0300
> init_ring() and refill_rx_ring() don't check if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
The transmit side has the same exact problem, can you take care of both
of them at the same time?
Thanks.
^ permalink raw reply
* Re: [PATCH net v2] net: mpls: Fix multipath selection for LSR use case
From: David Miller @ 2017-01-23 17:48 UTC (permalink / raw)
To: dsa; +Cc: netdev, rshearma, roopa
In-Reply-To: <1484945914-32085-1-git-send-email-dsa@cumulusnetworks.com>
From: David Ahern <dsa@cumulusnetworks.com>
Date: Fri, 20 Jan 2017 12:58:34 -0800
> MPLS multipath for LSR is broken -- always selecting the first nexthop
> in the one label case. For example:
>
> $ ip -f mpls ro ls
> 100
> nexthop as to 200 via inet 172.16.2.2 dev virt12
> nexthop as to 300 via inet 172.16.3.2 dev virt13
> 101
> nexthop as to 201 via inet6 2000:2::2 dev virt12
> nexthop as to 301 via inet6 2000:3::2 dev virt13
>
> In this example incoming packets have a single MPLS labels which means
> BOS bit is set. The BOS bit is passed from mpls_forward down to
> mpls_multipath_hash which never processes the hash loop because BOS is 1.
>
> Update mpls_multipath_hash to process the entire label stack. mpls_hdr_len
> tracks the total mpls header length on each pass (on pass N mpls_hdr_len
> is N * sizeof(mpls_shim_hdr)). When the label is found with the BOS set
> it verifies the skb has sufficient header for ipv4 or ipv6, and find the
> IPv4 and IPv6 header by using the last mpls_hdr pointer and adding 1 to
> advance past it.
>
> With these changes I have verified the code correctly sees the label,
> BOS, IPv4 and IPv6 addresses in the network header and icmp/tcp/udp
> traffic for ipv4 and ipv6 are distributed across the nexthops.
>
> Fixes: 1c78efa8319ca ("mpls: flow-based multipath selection")
> Acked-by: Robert Shearman <rshearma@brocade.com>
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> v2
> - rebase against net/master; v1 was mistakenly based against net-next
> - updated commit message based on Robert's comment about skipping the
> first label
Applied and queued up for -stable, thanks.
^ permalink raw reply
* iproute2: using .maxattr = IFLA_VLAN_MAX??
From: Murali Karicheri @ 2017-01-23 17:44 UTC (permalink / raw)
To: open list:TI NETCP ETHERNET DRIVER
Hi iproute2 experts,
Found the below usage of IFLA_VLAN_MAX in ip/iplink_hsr.c
struct link_util hsr_link_util = {
.id = "hsr",
.maxattr = IFLA_VLAN_MAX, <==== should be IFLA_HSR_MAX
.parse_opt = hsr_parse_opt,
.print_opt = hsr_print_opt,
.print_help = hsr_print_help,
};
This appears to be a cut-n-paste bug as the source is based on
ip/iplink_vlan.c and should be fixed to IFLA_HSR_MAX. If yes,
I will send a patch for this.
Thanks.
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* Re: EPOLLERR on memory mapped netlink socket
From: prashantkumar dhotre @ 2017-01-23 17:40 UTC (permalink / raw)
To: netdev
In-Reply-To: <CA+VDgmPKUnje8XTBx35kOsybwXnByUHYXwpZFjYw95V-8O1jbw@mail.gmail.com>
Appreciate any help on this
Thanks
On Mon, Jan 23, 2017 at 9:19 AM, prashantkumar dhotre
<prashantkumardhotre@gmail.com> wrote:
> Hi experts,
> I am new to netlink sockets.
> In my app , I am getting EPOLLERR in epoll_wait() on netlink socket
> continuously.
> epoll just notifies that there is a read event on socket (it does not
> tell if it is read or epollerr).
> What could be cause of this and what EPOLLERR on memory mapped netlink
> socket mean.is this Other side of netlink (kernel side) closed
> connection ?even if kernel side closed connection why non-stop
> repeated EPOLLERR s on netlink sockets ?
> What action should we take in such cases ? just close the socket or
> call getsockopt(SO_ERROR) to retrieve the pending error state from the
> socket and just continue without closing socket?
>
> how do we detect if kernel side closed the connection ?
> My understanding is :
> if we get read event notification from epoll on memory mapped netlink
> socket and in RX ring if the frame is neither NL_MMAP_STATUS_VALID and
> nor NL_MMAP_STATUS_COPY then we can conclude that this is a 'close()'
> from remote kernel socket and I can close connection by calling
> close() on my netlink socket.
> Is above understanding correct ?
>
> Please
^ permalink raw reply
* Re: wrong smp_mb__after_atomic() in tcp_check_space() ?
From: Oleg Nesterov @ 2017-01-23 17:18 UTC (permalink / raw)
To: Jason Baron
Cc: David S. Miller, Herbert Xu, Yauheni Kaliuta, netdev,
Eric Dumazet
In-Reply-To: <f5f9c542-2b23-9df9-098c-ad4007bf7e78@akamai.com>
On 01/23, Jason Baron wrote:
>
> >It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
> >and the patch looks correct in that we need the barriers in tcp_check_space()
> >and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?
> >
>
> Yes, I think it should be upgraded to an smp_mb() there. If you agree with
> this analysis, I will send a patch to upgrade it.
I think this makes sense, if nothing else to remove the obviously wrong
smp_mb__after_atomic() ;)
> Note, I did not actually
> run into this race in practice.
Yes, I am not sure it is actually necessary in practice, and even if
tcp_check_space() races with tcp_poll() and misses SOCK_NOSPACE it should
be probably called again later, but I know nothing about networking code.
We noticed this by accident, we have a bug report which really looks as a
missed wakeup in epoll, but this kernel is old and in particular it lacks
the commit 128dd1759 "epoll: prevent missed events on EPOLL_CTL_MOD" which
looks more promising. But we are not sure it can fully explain the problem
we can't reproduce, so we are looking for anything else which may contribute
to the problem.
Oleg.
^ permalink raw reply
* Re: [PATCH 1/3] powerpc: bpf: remove redundant check for non-null image
From: Naveen N. Rao @ 2017-01-23 17:14 UTC (permalink / raw)
To: David Miller; +Cc: ast, linuxppc-dev, daniel, netdev
In-Reply-To: <20170116.133859.1157069434816521683.davem@davemloft.net>
Hi David,
On 2017/01/16 01:38PM, David Miller wrote:
>
> I'm assuming these patches will go via the powerpc tree.
>
> If you want them to go into net-next, I kindly ask that you always
> explicitly say so, and furthermore always submit a patch series with
> a proper "[PATCH 0/N] ..." header posting.
Sure. Sorry for the confusion. I will be more explicit next time.
Thanks,
Naveen
^ permalink raw reply
* [PATCH] virtio_net: fix PAGE_SIZE > 64k
From: Michael S. Tsirkin @ 2017-01-23 17:06 UTC (permalink / raw)
To: linux-kernel; +Cc: Jason Wang, virtualization, netdev
I don't have any guests with PAGE_SIZE > 64k but the
code seems to be clearly broken in that case
as PAGE_SIZE / MERGEABLE_BUFFER_ALIGN will need
more than 8 bit and so the code in mergeable_ctx_to_buf_address
does not give us the actual true size.
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Lightly tested on x86 only.
drivers/net/virtio_net.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4a10500..5e3b07e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -48,8 +48,16 @@ module_param(gso, bool, 0444);
*/
DECLARE_EWMA(pkt_len, 1, 64)
+/* With mergeable buffers we align buffer address and use the low bits to
+ * encode its true size. Buffer size is up to 1 page so we need to align to
+ * square root of page size to ensure we reserve enough bits to encode the true
+ * size.
+ */
+#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT ((PAGE_SHIFT + 1) / 2)
+
/* Minimum alignment for mergeable packet buffers. */
-#define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, 256)
+#define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, \
+ 1U << MERGEABLE_BUFFER_MIN_ALIGN_SHIFT)
#define VIRTNET_DRIVER_VERSION "1.0.0"
--
MST
^ permalink raw reply related
* Re: [net PATCH v5 6/6] virtio_net: XDP support for adjust_head
From: Michael S. Tsirkin @ 2017-01-23 17:02 UTC (permalink / raw)
To: John Fastabend
Cc: Jason Wang, David Laight, john.r.fastabend@intel.com,
netdev@vger.kernel.org, alexei.starovoitov@gmail.com,
daniel@iogearbox.net
In-Reply-To: <5884319B.90909@gmail.com>
On Sat, Jan 21, 2017 at 08:14:19PM -0800, John Fastabend wrote:
> On 17-01-21 06:51 PM, Jason Wang wrote:
> >
> >
> > On 2017年01月21日 01:48, Michael S. Tsirkin wrote:
> >> On Fri, Jan 20, 2017 at 04:59:11PM +0000, David Laight wrote:
> >>> From: Michael S. Tsirkin
> >>>> Sent: 19 January 2017 21:12
> >>>>> On 2017?01?18? 23:15, Michael S. Tsirkin wrote:
> >>>>>> On Tue, Jan 17, 2017 at 02:22:59PM -0800, John Fastabend wrote:
> >>>>>>> Add support for XDP adjust head by allocating a 256B header region
> >>>>>>> that XDP programs can grow into. This is only enabled when a XDP
> >>>>>>> program is loaded.
> >>>>>>>
> >>>>>>> In order to ensure that we do not have to unwind queue headroom push
> >>>>>>> queue setup below bpf_prog_add. It reads better to do a prog ref
> >>>>>>> unwind vs another queue setup call.
> >>>>>>>
> >>>>>>> At the moment this code must do a full reset to ensure old buffers
> >>>>>>> without headroom on program add or with headroom on program removal
> >>>>>>> are not used incorrectly in the datapath. Ideally we would only
> >>>>>>> have to disable/enable the RX queues being updated but there is no
> >>>>>>> API to do this at the moment in virtio so use the big hammer. In
> >>>>>>> practice it is likely not that big of a problem as this will only
> >>>>>>> happen when XDP is enabled/disabled changing programs does not
> >>>>>>> require the reset. There is some risk that the driver may either
> >>>>>>> have an allocation failure or for some reason fail to correctly
> >>>>>>> negotiate with the underlying backend in this case the driver will
> >>>>>>> be left uninitialized. I have not seen this ever happen on my test
> >>>>>>> systems and for what its worth this same failure case can occur
> >>>>>>> from probe and other contexts in virtio framework.
> >>>>>>>
> >>>>>>> Signed-off-by: John Fastabend<john.r.fastabend@intel.com>
> >>>>>> I've been thinking about it - can't we drop
> >>>>>> old buffers without the head room which were posted before
> >>>>>> xdp attached?
> >>>>>>
> >>>>>> Avoiding the reset would be much nicer.
> >>>>>>
> >>>>>> Thoughts?
> >>>>>>
> >>>>> As been discussed before, device may use them in the same time so it's not
> >>>>> safe. Or do you mean detect them after xdp were set and drop the buffer
> >>>>> without head room, this looks sub-optimal.
> >>>>>
> >>>>> Thanks
> >>>> Yes, this is what I mean. Why is this suboptimal? It's a single branch
> >>>> in code. Yes we might lose some packets but the big hammer of device
> >>>> reset will likely lose more.
> >>> Why not leave let the hardware receive into the 'small' buffer (without
> >>> headroom) and do a copy when a frame is received.
> >>> Replace the buffers with 'big' ones for the next receive.
> >>> A data copy on a ring full of buffers won't really be noticed.
> >>>
> >>> David
> >>>
> >> I like that. John?
> >>
> >
> > This works, I prefer this only if it uses simpler code (but I suspect) than reset.
> >
> > Thanks
>
> Before the reset path I looked at doing this but it seems to require tracking
> if a buffer had headroom on a per buffer basis. I don't see a good spot to
> put a bit like this? It could go in the inbuf 'ctx' added by virtqueue_add_inbuf
> but I would need to change the current usage of ctx which in the mergeable case
> at least is just a simple pointer today. I don't like this because it
> complicates the normal path and the XDP hotpath.
>
> Otherwise we could somehow mark the ring at the point where XDP is enabled so
> that it can learn when a full iteration around the ring. But I can't see a
> simple way to make this work either.
>
> I don't know the reset look straight forward to me and although not ideal is
> fairly common on hardware based drivers during configuration changes. I'm open
> to any ideas on where to put the metadata to track headroom though.
>
> Thanks,
> John
Well with 4K pages we actually have 4 spare bits to use.
In fact this means we could reduce the mergeable buffer alignment.
It starts getting strange with 64K pages where we are
out of space, and I just noticed that with bigger pages
virtio is actually broken.
So let me fix it up first of all, and on top - maybe we can just
increase the alignment for 64k pages and up?
Truesize alignment to 512 is still reasonable and presumably
these 64k page boxes have lots of memory.
Would it make sense to tweak SK_RMEM_MAX up for larger
page sizes?
--
MST
^ permalink raw reply
* Re: [PATCH cumulus-4.1.y 2/5] vxlan: don't replace fdb entry if nothing changed
From: Stephen Hemminger @ 2017-01-23 17:02 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: davem, netdev, ramanb, jbenc, pshelar
In-Reply-To: <1484984410-3304-2-git-send-email-roopa@cumulusnetworks.com>
On Fri, 20 Jan 2017 23:40:07 -0800
Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> + if (!vxlan_addr_equal(&rd->remote_ip, ip) ||
> + rd->remote_port != port ||
> + rd->remote_vni != vni ||
> + rd->remote_ifindex != ifindex) {
> + dst_cache_reset(&rd->dst_cache);
> + rd->remote_ip = *ip;
> + rd->remote_port = port;
> + rd->remote_vni = vni;
> + rd->remote_ifindex = ifindex;
> + return 1;
> + }
> +
I think it would be clearer if negative logic was avoided.
if (vxlan_addr_equal(&rd->remote_ip, ip) &&
rd->remote_port == port &&
rd->remote_vni == vni &&
rd->ermote_ifindex == ifndex)
return 1;
dst_cache_reset ...
^ permalink raw reply
* Re: [PATCH cumulus-4.1.y 1/5] vxlan: flush fdb entries on oper down
From: Stephen Hemminger @ 2017-01-23 16:59 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: davem, netdev, ramanb, jbenc, pshelar
In-Reply-To: <1484984410-3304-1-git-send-email-roopa@cumulusnetworks.com>
On Fri, 20 Jan 2017 23:40:06 -0800
Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 19b1653..15b1c23 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -3276,6 +3276,12 @@ static int vxlan_netdevice_event(struct notifier_block *unused,
> vxlan_handle_lowerdev_unregister(vn, dev);
> else if (event == NETDEV_UDP_TUNNEL_PUSH_INFO)
> vxlan_push_rx_ports(dev);
> + else if (event == NETDEV_CHANGE) {
> + if (dev->netdev_ops == &vxlan_netdev_ops) {
> + if (netif_running(dev) && !netif_oper_up(dev))
> + vxlan_flush(netdev_priv(dev));
> + }
> + }
Looks correct.
Maybe logic would be clearer with a switch() statement here.
^ permalink raw reply
* [RFC PATCH net-next 1/5] skbuff: add stub to help computing crc32c on SCTP packets
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
sctp_compute_checksum requires crc32c symbol (provided by libcrc32c), so
it can't be used in net core. Like it has been done previously with other
symbols (e.g. ipv6_dst_lookup), introduce a stub struct skb_checksum_ops
to allow computation of SCTP checksum in net core after sctp.ko (and thus
libcrc32c) has been loaded.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/linux/skbuff.h | 2 ++
net/core/skbuff.c | 20 ++++++++++++++++++++
net/sctp/offload.c | 7 +++++++
3 files changed, 29 insertions(+)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6f63b7e..44fc804 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3119,6 +3119,8 @@ struct skb_checksum_ops {
__wsum (*combine)(__wsum csum, __wsum csum2, int offset, int len);
};
+extern const struct skb_checksum_ops *sctp_csum_stub __read_mostly;
+
__wsum __skb_checksum(const struct sk_buff *skb, int offset, int len,
__wsum csum, const struct skb_checksum_ops *ops);
__wsum skb_checksum(const struct sk_buff *skb, int offset, int len,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f8dbe4a..60e9963 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2235,6 +2235,26 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
}
EXPORT_SYMBOL(skb_copy_and_csum_bits);
+static __wsum warn_sctp_csum_update(const void *buff, int len, __wsum sum)
+{
+ net_warn_ratelimited("attempt to compute crc32c without sctp.ko\n");
+ return 0;
+}
+
+static __wsum warn_sctp_csum_combine(__wsum csum, __wsum csum2,
+ int offset, int len)
+{
+ net_warn_ratelimited("attempt to compute crc32c without sctp.ko\n");
+ return 0;
+}
+
+const struct skb_checksum_ops *sctp_csum_stub __read_mostly =
+ &(struct skb_checksum_ops) {
+ .update = warn_sctp_csum_update,
+ .combine = warn_sctp_csum_combine,
+};
+EXPORT_SYMBOL(sctp_csum_stub);
+
/**
* skb_zerocopy_headlen - Calculate headroom needed for skb_zerocopy()
* @from: source buffer
diff --git a/net/sctp/offload.c b/net/sctp/offload.c
index 7e869d0..1c9c548 100644
--- a/net/sctp/offload.c
+++ b/net/sctp/offload.c
@@ -98,6 +98,12 @@ static const struct net_offload sctp6_offload = {
},
};
+static const struct skb_checksum_ops *sctp_csum_ops __read_mostly =
+ &(struct skb_checksum_ops) {
+ .update = sctp_csum_update,
+ .combine = sctp_csum_combine,
+};
+
int __init sctp_offload_init(void)
{
int ret;
@@ -110,6 +116,7 @@ int __init sctp_offload_init(void)
if (ret)
goto ipv4;
+ sctp_csum_stub = sctp_csum_ops;
return ret;
ipv4:
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 5/5] Documentation: add description of skb_sctp_csum_help
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
Add description of skb_sctp_csum_help in networking/checksum-offload.txt;
while at it, remove reference to skb_csum_off_chk* functions, since they
are not present anymore in Linux since commit cf53b1da73bd ('Revert "net:
Add driver helper functions to determine checksum"').
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
Documentation/networking/checksum-offloads.txt | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/Documentation/networking/checksum-offloads.txt b/Documentation/networking/checksum-offloads.txt
index 56e3686..cb7a7e5 100644
--- a/Documentation/networking/checksum-offloads.txt
+++ b/Documentation/networking/checksum-offloads.txt
@@ -49,9 +49,9 @@ A driver declares its offload capabilities in netdev->hw_features; see
and csum_offset given in the SKB; if it tries to deduce these itself in
hardware (as some NICs do) the driver should check that the values in the
SKB match those which the hardware will deduce, and if not, fall back to
- checksumming in software instead (with skb_checksum_help or one of the
- skb_csum_off_chk* functions as mentioned in include/linux/skbuff.h). This
- is a pain, but that's what you get when hardware tries to be clever.
+ checksumming in software instead (with skb_checksum_help or
+ skb_sctp_csum_help functions as mentioned in include/linux/skbuff.h).
+ This is a pain, but that's what you get when hardware tries to be clever.
The stack should, for the most part, assume that checksum offload is
supported by the underlying device. The only place that should check is
@@ -60,7 +60,8 @@ The stack should, for the most part, assume that checksum offload is
may include other offloads besides TX Checksum Offload) and, if they are
not supported or enabled on the device (determined by netdev->features),
performs the corresponding offload in software. In the case of TX
- Checksum Offload, that means calling skb_checksum_help(skb).
+ Checksum Offload, that means calling skb_sctp_csum_help(skb) for SCTP
+ packets, and skb_checksum_help(skb) for other packets.
LCO: Local Checksum Offload
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 0/5] net: improve support for SCTP checksums
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
When NETIF_F_CSUM_MASK bits are all 0 on netdev features, validate_xmit_skb
uses skb_checksum_help to compute 16-bit, 2-complement checksum on non-GSO
skbs having ip_summed equal to CHECKSUM_PARTIAL.
This results in a systematic corruption of SCTP packets, since they need
to be checksummed with crc32c. Moreover, this is done regardless the value
of NETIF_F_SCTP_CRC, so any chance to offload crc32c computation on the
NIC is lost. Finally, even when at least one bit in NETIF_F_CSUM_MASK is
set on netdev features, validate_xmit_skb skips checksum computation - but
then most NIC drivers can only call skb_checksum_help if their HW can't
offload the checksum computation. Depending on the driver code, this
results in wrong handling of SCTP, leading to:
- packet being dropped
- packet being transmitted with identically-zero checksum
- packet being transmitted with 2-complement checksum instead of crc32c
This series tries to address the above issue, by providing:
- the possibility to compute crc32c on skbs in Linux net core [patch 1]
- skb_sctp_csum_help, a function sharing common code with the original
skb_checksum_help, that performs SW checksumming for skbs using crc32c
[patch 2 and patch 3]
- skb_csum_hwoffload_help, called by validate xmit skb to perform SW
checksumming using the correct algorithm based on the value of IP
protocol number and netdev features bitmask [patch 4]
- an update to Linux documentation [patch 5]
Davide Caratti (5):
skbuff: add stub to help computing crc32c on SCTP packets
net: split skb_checksum_help
net: introduce skb_sctp_csum_help
net: more accurate checksumming in validate_xmit_skb
Documentation: add description of skb_sctp_csum_help
Documentation/networking/checksum-offloads.txt | 9 +-
include/linux/netdevice.h | 1 +
include/linux/skbuff.h | 5 +-
net/core/dev.c | 132 +++++++++++++++++++++----
net/core/skbuff.c | 20 ++++
net/sctp/offload.c | 7 ++
6 files changed, 151 insertions(+), 23 deletions(-)
--
2.7.4
^ permalink raw reply
* [RFC PATCH net-next 4/5] net: more accurate checksumming in validate_xmit_skb
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
introduce skb_csum_hwoffload_help and use it as a replacement for
skb_checksum_help in validate_xmit_skb, to compute checksum using crc32c or
2-complement Internet Checksum (or leave the packet unchanged and let the
NIC do the checksum), depending on netdev checksum offloading capabilities
and on presence of IPPROTO_SCTP as protocol number in IPv4/IPv6 header.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/core/dev.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 50 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 45cee84..f8cb3ba 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -140,6 +140,7 @@
#include <linux/hrtimer.h>
#include <linux/netfilter_ingress.h>
#include <linux/crash_dump.h>
+#include <linux/sctp.h>
#include "net-sysfs.h"
@@ -2960,6 +2961,54 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb,
return skb;
}
+static int skb_csum_hwoffload_help(struct sk_buff *skb,
+ netdev_features_t features)
+{
+ bool encap = skb->encapsulation;
+ unsigned int offset = 0;
+ __be16 protocol;
+
+ if (likely((features & (NETIF_F_SCTP_CRC | NETIF_F_CSUM_MASK)) ==
+ (NETIF_F_SCTP_CRC | NETIF_F_CSUM_MASK)))
+ return 0;
+
+ if (skb->csum_offset != offsetof(struct sctphdr, checksum))
+ goto inet_csum;
+
+ if (encap) {
+ protocol = skb->inner_protocol;
+ if (skb->inner_protocol_type == ENCAP_TYPE_IPPROTO)
+ switch (protocol) {
+ case IPPROTO_IPV6:
+ protocol = ntohs(ETH_P_IPV6);
+ break;
+ case IPPROTO_IP:
+ protocol = ntohs(ETH_P_IP);
+ break;
+ default:
+ goto inet_csum;
+ }
+ } else {
+ protocol = vlan_get_protocol(skb);
+ }
+ switch (protocol) {
+ case ntohs(ETH_P_IP):
+ if ((encap ? inner_ip_hdr(skb) : ip_hdr(skb))->protocol ==
+ IPPROTO_SCTP)
+ goto sctp_csum;
+ break;
+ case ntohs(ETH_P_IPV6):
+ if (ipv6_find_hdr(skb, &offset, IPPROTO_SCTP, NULL, NULL) ==
+ IPPROTO_SCTP)
+ goto sctp_csum;
+ break;
+ }
+inet_csum:
+ return !(features & NETIF_F_CSUM_MASK) ? skb_checksum_help(skb) : 0;
+sctp_csum:
+ return !(features & NETIF_F_SCTP_CRC) ? skb_sctp_csum_help(skb) : 0;
+}
+
static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
{
netdev_features_t features;
@@ -2995,8 +3044,7 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device
else
skb_set_transport_header(skb,
skb_checksum_start_offset(skb));
- if (!(features & NETIF_F_CSUM_MASK) &&
- skb_checksum_help(skb))
+ if (skb_csum_hwoffload_help(skb, features))
goto out_kfree_skb;
}
}
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 3/5] net: introduce skb_sctp_csum_help
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
skb_sctp_csum_help is like skb_checksum_help, but it is designed for
checksumming SCTP packets using crc32c (see RFC3309), provided that
sctp.ko has been loaded before. In case sctp.ko is not loaded, invoking
skb_sctp_csum_help() on a skb results in the following printout:
sk_buff: attempt to compute crc32c without sctp.ko
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/linux/netdevice.h | 1 +
include/linux/skbuff.h | 3 ++-
net/core/dev.c | 29 +++++++++++++++++++++++++++++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3868c32..9d72824 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3902,6 +3902,7 @@ void netdev_rss_key_fill(void *buffer, size_t len);
int dev_get_nest_level(struct net_device *dev);
int skb_checksum_help(struct sk_buff *skb);
+int skb_sctp_csum_help(struct sk_buff *skb);
struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
netdev_features_t features, bool tx_path);
struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 44fc804..91b4e22 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -192,7 +192,8 @@
* accordingly. Note the there is no indication in the skbuff that the
* CHECKSUM_PARTIAL refers to an SCTP checksum, a driver that supports
* both IP checksum offload and SCTP CRC offload must verify which offload
- * is configured for a packet presumably by inspecting packet headers.
+ * is configured for a packet presumably by inspecting packet headers; in
+ * case, skb_sctp_csum_help is provided to compute CRC on SCTP packets.
*
* NETIF_F_FCOE_CRC - This feature indicates that a device is capable of
* offloading the FCOE CRC in a packet. To perform this offload the stack
diff --git a/net/core/dev.c b/net/core/dev.c
index 6742160..45cee84 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2554,6 +2554,29 @@ static int skb_rfc1624_csum(struct sk_buff *skb, int offset)
return ret;
}
+/* compute 32-bit RFC3309 checksum and store it at skb->data + offset */
+static int skb_rfc3309_csum(struct sk_buff *skb, int offset)
+{
+ __le32 crc32c_csum;
+ int ret = 0;
+
+ crc32c_csum = cpu_to_le32(~__skb_checksum(skb, offset,
+ skb->len - offset, ~(__u32)0,
+ sctp_csum_stub));
+ offset += skb->csum_offset;
+ BUG_ON((offset + sizeof(__le32)) > skb_headlen(skb));
+
+ if (skb_cloned(skb) &&
+ !skb_clone_writable(skb, offset + sizeof(__le32))) {
+ ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
+ if (ret)
+ goto out;
+ }
+ *(__le32 *)(skb->data + offset) = crc32c_csum;
+out:
+ return ret;
+}
+
/* Invalidate hardware checksum when packet is to be mangled, and
* complete checksum manually on outgoing path.
* @skb - buffer that needs checksum
@@ -2599,6 +2622,12 @@ int skb_checksum_help(struct sk_buff *skb)
}
EXPORT_SYMBOL(skb_checksum_help);
+int skb_sctp_csum_help(struct sk_buff *skb)
+{
+ return __skb_checksum_help(skb, skb_rfc3309_csum);
+}
+EXPORT_SYMBOL(skb_sctp_csum_help);
+
__be16 skb_network_protocol(struct sk_buff *skb, int *depth)
{
__be16 type = skb->protocol;
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 2/5] net: split skb_checksum_help
From: Davide Caratti @ 2017-01-23 16:52 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-sctp, Marcelo Ricardo Leitner
In-Reply-To: <cover.1485177252.git.dcaratti@redhat.com>
skb_checksum_help is designed to compute the Internet Checksum only. To
avoid duplicating code when other checksumming algorithms (e.g. crc32c)
are used, separate common part from RFC1624-specific part.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/core/dev.c | 51 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index ad5959e..6742160 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2532,13 +2532,36 @@ static void skb_warn_bad_offload(const struct sk_buff *skb)
skb_shinfo(skb)->gso_type, skb->ip_summed);
}
-/*
- * Invalidate hardware checksum when packet is to be mangled, and
+/* compute 16-bit RFC1624 checksum and store it at skb->data + offset */
+static int skb_rfc1624_csum(struct sk_buff *skb, int offset)
+{
+ __wsum csum;
+ int ret = 0;
+
+ csum = skb_checksum(skb, offset, skb->len - offset, 0);
+
+ offset += skb->csum_offset;
+ BUG_ON(offset + sizeof(__sum16) > skb_headlen(skb));
+
+ if (skb_cloned(skb) &&
+ !skb_clone_writable(skb, offset + sizeof(__sum16))) {
+ ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
+ if (ret)
+ goto out;
+ }
+ *(__sum16 *)(skb->data + offset) = csum_fold(csum) ?: CSUM_MANGLED_0;
+out:
+ return ret;
+}
+
+/* Invalidate hardware checksum when packet is to be mangled, and
* complete checksum manually on outgoing path.
+ * @skb - buffer that needs checksum
+ * @csum_algo(skb, offset) - function used to compute the checksum
*/
-int skb_checksum_help(struct sk_buff *skb)
+static int __skb_checksum_help(struct sk_buff *skb,
+ int (*csum_algo)(struct sk_buff *, int))
{
- __wsum csum;
int ret = 0, offset;
if (skb->ip_summed == CHECKSUM_COMPLETE)
@@ -2560,24 +2583,20 @@ int skb_checksum_help(struct sk_buff *skb)
offset = skb_checksum_start_offset(skb);
BUG_ON(offset >= skb_headlen(skb));
- csum = skb_checksum(skb, offset, skb->len - offset, 0);
-
- offset += skb->csum_offset;
- BUG_ON(offset + sizeof(__sum16) > skb_headlen(skb));
-
- if (skb_cloned(skb) &&
- !skb_clone_writable(skb, offset + sizeof(__sum16))) {
- ret = pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
- if (ret)
- goto out;
- }
- *(__sum16 *)(skb->data + offset) = csum_fold(csum) ?: CSUM_MANGLED_0;
+ ret = csum_algo(skb, offset);
+ if (ret)
+ goto out;
out_set_summed:
skb->ip_summed = CHECKSUM_NONE;
out:
return ret;
}
+
+int skb_checksum_help(struct sk_buff *skb)
+{
+ return __skb_checksum_help(skb, skb_rfc1624_csum);
+}
EXPORT_SYMBOL(skb_checksum_help);
__be16 skb_network_protocol(struct sk_buff *skb, int *depth)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 0/4] Make xfrm usable by 32-bit programs
From: David Miller @ 2017-01-23 16:57 UTC (permalink / raw)
To: David.Laight
Cc: cernekee, steffen.klassert, herbert, paul, sds, eparis,
linux-kernel, netdev, selinux, fw, fan.du, dianders, dtor
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026BC58@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Mon, 23 Jan 2017 16:45:39 +0000
> Provided you've got the length of the user's buffer the compat code
> ought to be trivial (if tedious).
Wireless guys had to deal with a similar problem with nl80211.
You don't know who is going to get the message when you build it,
because you can't possibly know what applications are listening
on the netlink sockets for notifications and even if you did
that set changes dynamically and could be different by the time
the notification is delivered.
The long and short of it is that you must always generate both the
native 64-bit as well as the 32-bit compat version of every message,
then at delivery time you enqueue the appropriate one to each
receiving socket.
This is how nl80211 solved the problem, and it's what XFRM should do
as well.
^ permalink raw reply
* Re: wrong smp_mb__after_atomic() in tcp_check_space() ?
From: Jason Baron @ 2017-01-23 16:56 UTC (permalink / raw)
To: Oleg Nesterov, David S. Miller
Cc: Herbert Xu, Yauheni Kaliuta, netdev, Eric Dumazet
In-Reply-To: <20170123143025.GA31676@redhat.com>
On 01/23/2017 09:30 AM, Oleg Nesterov wrote:
> Hello,
>
> smp_mb__after_atomic() looks wrong and misleading, sock_reset_flag() does the
> non-atomic __clear_bit() and thus it can not guarantee test_bit(SOCK_NOSPACE)
> (non-atomic too) won't be reordered.
>
Indeed. Here's a bit of discussion on it:
http://marc.info/?l=linux-netdev&m=146662325920596&w=2
> It was added by 3c7151275c0c9a "tcp: add memory barriers to write space paths"
> and the patch looks correct in that we need the barriers in tcp_check_space()
> and tcp_poll() in theory, so it seems tcp_check_space() needs smp_mb() ?
>
Yes, I think it should be upgraded to an smp_mb() there. If you agree
with this analysis, I will send a patch to upgrade it. Note, I did not
actually run into this race in practice.
Thanks,
-Jason
^ permalink raw reply
* Re: [PATCH net-next] net: ipv6: ignore null_entry on route dumps
From: David Ahern @ 2017-01-23 16:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, xiaolong.ye
In-Reply-To: <20170122.235829.1911961579892347071.davem@davemloft.net>
On 1/22/17 9:58 PM, David Miller wrote:
>
> David, please slow down.
>
> How is the NULL entry getting selected to be dumped and passed
> down here in the first place?
The null_entry might be a 'marker' but is integral to the ipv6 fib. For starters:
static int __net_init fib6_net_init(struct net *net)
{
...
net->ipv6.fib6_main_tbl = kzalloc(sizeof(*net->ipv6.fib6_main_tbl),
GFP_KERNEL);
...
net->ipv6.fib6_main_tbl->tb6_id = RT6_TABLE_MAIN;
net->ipv6.fib6_main_tbl->tb6_root.leaf = net->ipv6.ip6_null_entry;
...
Same goes for every other table allocated:
static struct fib6_table *fib6_alloc_table(struct net *net, u32 id)
{
struct fib6_table *table;
table = kzalloc(sizeof(*table), GFP_ATOMIC);
if (table) {
table->tb6_id = id;
table->tb6_root.leaf = net->ipv6.ip6_null_entry;
..
}
And if you look at the fib6_add code:
sfn->leaf = info->nl_net->ipv6.ip6_null_entry;
atomic_inc(&info->nl_net->ipv6.ip6_null_entry->rt6i_ref);
sfn->fn_flags = RTN_ROOT;
sfn->fn_sernum = sernum;
/* Now add the first leaf node to new subtree */
sn = fib6_add_1(sfn, &rt->rt6i_src.addr,
rt->rt6i_src.plen,
offsetof(struct rt6_info, rt6i_src),
allow_create, replace_required, sernum);
Meaning that a walk of the table is going to dump the ip6_null_entry route, potentially multiple times.
As far as the dump request the chain rtnetlink message invokes inet6_dump_fib:
inet6_dump_fib
- fib6_dump_table
+ fib6_walk
* fib6_walk_continue
- fib6_dump_node
+ rt6_dump_route
Seems to me the most appropriate place to check that rt is the null_entry route is rt6_dump_route but perhaps fib6_dump_node could as well. This patch buts the check in rt6_dump_route.
^ permalink raw reply
* RE: [PATCH 0/4] Make xfrm usable by 32-bit programs
From: David Laight @ 2017-01-23 16:45 UTC (permalink / raw)
To: 'Kevin Cernekee', steffen.klassert@secunet.com,
herbert@gondor.apana.org.au, davem@davemloft.net,
paul@paul-moore.com, sds@tycho.nsa.gov, eparis@parisplace.org
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
selinux@tycho.nsa.gov, fw@strlen.de, fan.du@windriver.com,
dianders@chromium.org, dtor@chromium.org
In-Reply-To: <20170121000507.34381-1-cernekee@chromium.org>
From: Kevin Cernekee
> Sent: 21 January 2017 00:05
> Several of the xfrm netlink and setsockopt() interfaces are not usable
> from a 32-bit binary running on a 64-bit kernel due to struct padding
> differences. This has been the case for many, many years[0]. This
> patch series deprecates the broken netlink messages and replaces them
> with packed structs that are compatible between 64-bit and 32-bit
> programs.
Do you mean '__packed' or just structures with no holes?
The latter is really the best.
You can add a compile time assert on the structure sizes to ensure
that the user API is never accidentally broken.
I can't imagine any reason to change the 64bit structures except
(maybe) to make any padding explicit.
> It retains support for legacy user programs (i.e. anything
> that is currently working today), and allows legacy support to be
> compiled out via CONFIG_XFRM_USER_LEGACY if it becomes unnecessary in
> the future.
...
Isn't that effectively the same as doing a compat layer?
Otherwise you can't build a 32bit app that will work with an 'old'
32bit kernel and a new 64bit one.
Provided you've got the length of the user's buffer the compat code
ought to be trivial (if tedious).
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox