* Re: [PATCH] isdn: eicon: free pointer after using it in log msg in divas_um_idi_delete_entity()
From: David Miller @ 2013-10-02 20:01 UTC (permalink / raw)
To: jj; +Cc: mac, isdn, netdev, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1309302119550.4992@swampdragon.chaosbits.net>
From: Jesper Juhl <jj@chaosbits.net>
Date: Mon, 30 Sep 2013 21:25:27 +0200 (CEST)
> Not really a problem, but nice IMHO; the Coverity static analyzer
> complains that we use the pointer 'e' after it has been freed, so move
> the freeing below the final use, even if that use is just using the
> value of the pointer and not actually dereferencing it.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [patch net] udp6: respect IPV6_DONTFRAG sockopt in case there are pending frames
From: David Miller @ 2013-10-02 20:00 UTC (permalink / raw)
To: hannes; +Cc: jiri, netdev, kuznet, jmorris, kaber, yoshfuji
In-Reply-To: <20131001220354.GK10771@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Wed, 2 Oct 2013 00:03:54 +0200
> Hi David!
>
> On Mon, Sep 30, 2013 at 11:21:55PM +0200, Hannes Frederic Sowa wrote:
>> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>
> (for patchwork correspondence)
>
> I still think this patch is perfectly fine but it opens up a hole in the
> ip6_append_data logic where it is possible to crash the kernel. Jiri and me
> will look after it in the thread " ipv6: udp packets following an UFO enqueued
> packet need also be handled by UFO".
>
> So, for the time being, I would like to withdraw my Acked-by until this is
> sorted out.
Jiri please repost the IPV6_DONTFRAG patch once things are sorted, thanks.
^ permalink raw reply
* Re: [PATCH net-next] xen-netfront: convert to GRO API
From: David Miller @ 2013-10-02 19:54 UTC (permalink / raw)
To: wei.liu2; +Cc: netdev, xen-devel, konrad.wilk, abchak, ian.campbell
In-Reply-To: <1380545194-13419-1-git-send-email-wei.liu2@citrix.com>
From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 30 Sep 2013 13:46:34 +0100
> Anirban was seeing netfront received MTU size packets, which downgraded
> throughput. The following patch makes netfront use GRO API which
> improves throughput for that case.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Anirban Chakraborty <abchak@juniper.net>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Konrad Wilk <konrad.wilk@oracle.com>
Applied, to net-next.
Anirban suggested to queue this to -stable but that is absolutely
not appropriate so I'm not doing that.
^ permalink raw reply
* Re: [PATCH 3/3] tg3: use phylib when robo switch is in use
From: David Miller @ 2013-10-02 19:42 UTC (permalink / raw)
To: hauke; +Cc: nsujir, mchan, netdev
In-Reply-To: <1380402928-11480-3-git-send-email-hauke@hauke-m.de>
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 28 Sep 2013 23:15:28 +0200
> When a switch is connected as a PHY to the MAC driven by tg3, use
> phylib and provide the phy address to tg3 from the sprom.
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 2/3] ssb: provide phy address for Gigabit Ethernet driver
From: David Miller @ 2013-10-02 19:42 UTC (permalink / raw)
To: hauke; +Cc: nsujir, mchan, netdev
In-Reply-To: <1380402928-11480-2-git-send-email-hauke@hauke-m.de>
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 28 Sep 2013 23:15:27 +0200
> Add a function to provide the phy address which should be used to the
> Gigabit Ethernet driver connected to ssb.
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH 1/3] tg3: add support a phy at an address different than 01
From: David Miller @ 2013-10-02 19:42 UTC (permalink / raw)
To: hauke; +Cc: nsujir, mchan, netdev
In-Reply-To: <1380402928-11480-1-git-send-email-hauke@hauke-m.de>
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 28 Sep 2013 23:15:26 +0200
> When phylib was in use tg3 only searched at address 01 on the mdio
> bus and did not work with any other address. On the BCM4705 SoCs the
> switch is connected as a PHY behind the MAC driven by tg3 and it is at
> PHY address 30 in most cases. This is a preparation patch to allow
> support for such switches.
>
> phy_addr is set to TG3_PHY_MII_ADDR for all devices, which are using
> phylib, so this should not change any behavior.
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH RFC] random: introduce get_random_bytes_busy_wait_initialized
From: Theodore Ts'o @ 2013-10-02 19:40 UTC (permalink / raw)
To: Eric Dumazet, Tom Herbert, davem, netdev, jesse.brandeburg,
linux-kernel
In-Reply-To: <20131002171839.GQ10771@order.stressinduktion.org>
On Wed, Oct 02, 2013 at 07:18:40PM +0200, Hannes Frederic Sowa wrote:
>
> I agree. I will look if this is easily possible for secure_seq and
> syncookies but depending on the data structure and its size it is a much
> harder thing to do. I wanted to try the low-hanging fruits first. ;)
To use syncookies as an example, you shouldn't need to store all of
the old syncookies. Instead, if every 10 minutes or so, you rekey,
and you keep both the old and the new secrets around, you could just
simply check an incoming TCP packet using first the new key, and then
the old key. During the transition window it would take a wee bit
more CPU time, but most of the time, it wouldn't cost anything extra
in CPU time, and the only extra cost is the space for the old key.
>From a security perspective it would be much better if we tried to
make all of the places where we draw randomness and somehow try to
rekey on a periodic basis. That way, even if the initial value isn't
super-secure, that situation will heal itself fairly rapidly. And it
also means that even if an adversary can brute-force break a 32-bit
secret, they would have to do so within 5 or 10 minutes in order for
it to be useful, and even if they could, it would only be useful for a
short window of time.
I know it won't always be possible, but to the extent that we can do
this, it would be a big improvement from a security perspective.
Cheers,
- Ted
^ permalink raw reply
* Re: [PATCH RFC 36/77] ipr: Enable MSI-X when IPR_USE_MSIX type is set, not IPR_USE_MSI
From: Brian King @ 2013-10-02 19:31 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-kernel, linux-mips, VMware, Inc., linux-nvme, linux-ide,
stable, linux-s390, Andy King, linux-scsi, linux-rdma, x86,
linux-pci, iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Jon Mason, Ingo Molnar,
Solarflare linux maintainers, netdev, Ralf Baechle, e1000-devel,
Martin Schwidefsky, linux390, linuxppc-dev, Wendy Xiong
In-Reply-To: <2d4272136269f3cb3b1a5ac53b12aa45c7ea65c0.1380703263.git.agordeev@redhat.com>
On 10/02/2013 05:48 AM, Alexander Gordeev wrote:
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
> drivers/scsi/ipr.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
> index fb57e21..762a93e 100644
> --- a/drivers/scsi/ipr.c
> +++ b/drivers/scsi/ipr.c
> @@ -9527,7 +9527,7 @@ static int ipr_probe_ioa(struct pci_dev *pdev,
> ipr_number_of_msix = IPR_MAX_MSIX_VECTORS;
> }
>
> - if (ioa_cfg->ipr_chip->intr_type == IPR_USE_MSI &&
> + if (ioa_cfg->ipr_chip->intr_type == IPR_USE_MSIX &&
> ipr_enable_msix(ioa_cfg) == 0)
> ioa_cfg->intr_flag = IPR_USE_MSIX;
> else if (ioa_cfg->ipr_chip->intr_type == IPR_USE_MSI &&
>
Nack. ioa_cfg->ipr_chip->intr_type gets initialized from the ipr_chip table
at the top of the driver which never sets intr_type to IPR_USE_MSIX, so this
will break MSI-X support for ipr.
We indicate at the chip level only whether we want to force LSI or whether
we want to enable MSI / MSI-X and then try enabling MSI-X and fall back to
MSI if MSI-X is not available or does not work. We then set intr_flag to indicate
what we are actually using on the specific adapter.
Thanks,
Brian
--
Brian King
Power Linux I/O
IBM Linux Technology Center
^ permalink raw reply
* [patch] net: heap overflow in __audit_sockaddr()
From: Dan Carpenter @ 2013-10-02 18:58 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, security, Jüri Aedla
In-Reply-To: <20130319.095547.1333124517060824574.davem@davemloft.net>
We need to cap ->msg_namelen or it leads to a buffer overflow when we
to the memcpy() in __audit_sockaddr(). It requires CAP_AUDIT_CONTROL to
exploit this bug.
The call tree is:
___sys_recvmsg()
move_addr_to_user()
audit_sockaddr()
__audit_sockaddr()
Reported-by: Jüri Aedla <juri.aedla@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/socket.c b/net/socket.c
index ebed4b6..c226ace 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1964,6 +1964,16 @@ struct used_address {
unsigned int name_len;
};
+static int copy_msghdr_from_user(struct msghdr *kmsg,
+ struct msghdr __user *umsg)
+{
+ if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
+ return -EFAULT;
+ if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
+ return -EINVAL;
+ return 0;
+}
+
static int ___sys_sendmsg(struct socket *sock, struct msghdr __user *msg,
struct msghdr *msg_sys, unsigned int flags,
struct used_address *used_address)
@@ -1982,8 +1992,11 @@ static int ___sys_sendmsg(struct socket *sock, struct msghdr __user *msg,
if (MSG_CMSG_COMPAT & flags) {
if (get_compat_msghdr(msg_sys, msg_compat))
return -EFAULT;
- } else if (copy_from_user(msg_sys, msg, sizeof(struct msghdr)))
- return -EFAULT;
+ } else {
+ err = copy_msghdr_from_user(msg_sys, msg);
+ if (err)
+ return err;
+ }
if (msg_sys->msg_iovlen > UIO_FASTIOV) {
err = -EMSGSIZE;
@@ -2191,8 +2204,11 @@ static int ___sys_recvmsg(struct socket *sock, struct msghdr __user *msg,
if (MSG_CMSG_COMPAT & flags) {
if (get_compat_msghdr(msg_sys, msg_compat))
return -EFAULT;
- } else if (copy_from_user(msg_sys, msg, sizeof(struct msghdr)))
- return -EFAULT;
+ } else {
+ err = copy_msghdr_from_user(msg_sys, msg);
+ if (err)
+ return err;
+ }
if (msg_sys->msg_iovlen > UIO_FASTIOV) {
err = -EMSGSIZE;
^ permalink raw reply related
* Re: [PATCH RFC 04/77] PCI/MSI/s390: Remove superfluous check of MSI type
From: Greg KH @ 2013-10-02 18:17 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Bjorn Helgaas, Ralf Baechle,
Michael Ellerman, Benjamin Herrenschmidt, Martin Schwidefsky,
Ingo Molnar, Tejun Heo, Dan Williams, Andy King, Jon Mason,
Matt Porter, stable-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
linux390-tA70FqPdS9bQT0dZR+AlfA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, x86-DgEjT+Ai2ygdnm+yROfE0A,
linux-ide-u79uwXL29TY76Z2rM5mHXA, iss_storagedev-VXdhtT5mjnY,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-driver-h88ZbnxC6KDQT0dZR+AlfA, Solarflare linux maintainers
In-Reply-To: <bae65aa3e30dfd23bd5ed47add7310cfbb96243a.1380703262.git.agordeev-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Wed, Oct 02, 2013 at 12:48:20PM +0200, Alexander Gordeev wrote:
> arch_setup_msi_irqs() hook can only be called from the generic
> MSI code which ensures correct MSI type parameter.
>
> Signed-off-by: Alexander Gordeev <agordeev-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> arch/s390/pci/pci.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
I have no idea why you sent the stable@ alias so many patches, all in
the incorrect form for them to be ever accepted in the stable kernel
releases. Please read Documentation/stable_kernel_rules.txt for how to
do this properly.
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net-next 2/3] udp: Add udp early demux
From: Eric Dumazet @ 2013-10-02 18:09 UTC (permalink / raw)
To: Shawn Bohrer; +Cc: David Miller, tomk, netdev
In-Reply-To: <20131002173422.GA7824@sbohrermbp13-local.rgmadvisors.com>
On Wed, 2013-10-02 at 12:34 -0500, Shawn Bohrer wrote:
> On Tue, Oct 01, 2013 at 01:52:49PM -0700, Eric Dumazet wrote:
> > On Tue, 2013-10-01 at 14:33 -0500, Shawn Bohrer wrote:
> > > The removal of the routing cache introduced a performance regression for
> > > some UDP workloads since a dst lookup must be done for each packet.
> > > This change caches the dst per socket in a similar manner to what we do
> > > for TCP by implementing early_demux.
> > >
> > > For UDP multicast we can only cache the dst if there is only one
> > > receiving socket on the host. Since caching only works when there is
> > > one receiving socket we do the multicast socket lookup using RCU.
> >
> > For unicast, we should find a matching socket for early demux only if
> > this is a connected socket.
> >
> > Otherwise, forwarding setups will break.
> >
> > You probably need to add a minimum score to __udp4_lib_lookup()
>
> Perhaps I'm missing something but I don't think a minimum score would
> work because compute_score() and compute_score2() have several ways of
> returning a score of lets say 4 and I don't think they all mean the
> socket is connected.
Just change how score is computed. The existing +4 values are not hard
coded anywhere.
You want to compute a score so that a single compare against a threshold
is enough to tell you what's going on, before even taking a refcount on
the socket.
> Why not just test the socket returned by
> __udp4_lib_lookup() to see if it is connected in
> udp_v4_early_demux()? Something like:
>
> sk = __udp4_lib_lookup(net, iph->saddr, uh->source,
> iph->daddr, uh->dest, dif,
> &udp_table);
> /* Only demux connected sockets or forwarding setups will break */
> if (sk && !inet_sk(sk)->inet_daddr)
> return;
nice socket refcount leak ;)
^ permalink raw reply
* Re: [PATCH v2.41 5/5] datapath: Add basic MPLS support to kernel
From: Pravin Shelar @ 2013-10-02 18:03 UTC (permalink / raw)
To: Simon Horman
Cc: dev@openvswitch.org, netdev, Jesse Gross, Ben Pfaff, Ravi K,
Isaku Yamahata, Joe Stringer
In-Reply-To: <1380610064-14856-6-git-send-email-horms@verge.net.au>
On Mon, Sep 30, 2013 at 11:47 PM, Simon Horman <horms@verge.net.au> wrote:
> Allow datapath to recognize and extract MPLS labels into flow keys
> and execute actions which push, pop, and set labels on packets.
>
> Based heavily on work by Leo Alterman, Ravi K, Isaku Yamahata and Joe Stringer.
>
> Cc: Ravi K <rkerur@gmail.com>
> Cc: Leo Alterman <lalterman@nicira.com>
> Cc: Isaku Yamahata <yamahata@valinux.co.jp>
> Cc: Joe Stringer <joe@wand.net.nz>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
>
> +
> + /* this hack needed to get regular skb_gso_segment() */
> +#ifdef HAVE___SKB_GSO_SEGMENT
> +#undef __skb_gso_segment
> + skb_gso = __skb_gso_segment(skb, features, tx_path);
> +#else
> +#undef skb_gso_segment
> + skb_gso = skb_gso_segment(skb, features);
> +#endif
> +
We can get rid of #ifdefs by just using different name for
rpl___skb_gso_segment(), something like mpls_vlan_skb_gso_segment().
The way it is done for tnl-gso.
^ permalink raw reply
* Re: [PATCH net-next] net:drivers/net: Miscellaneous conversions to ETH_ALEN
From: Joe Perches @ 2013-10-02 18:02 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: Julia Lawall, Kalle Valo, netdev, ath10k
In-Reply-To: <CAB=NE6V3b4+B06hzPjhww52wp1FHho3329cP25=5Cy833bysvg@mail.gmail.com>
On Wed, 2013-10-02 at 10:44 -0700, Luis R. Rodriguez wrote:
> On Tue, Oct 1, 2013 at 11:40 PM, Joe Perches <joe@perches.com> wrote:
> > Please include netdev. (cc'd)
> >
> >> Joe Perches <joe@perches.com> writes:
> >>
> >> > Convert the memset/memcpy uses of 6 to ETH_ALEN
> >> > where appropriate.
> >
> >> > Signed-off-by: Joe Perches <joe@perches.com>
>
> I think these sorts of patches are good -- but once applied it'd be
> good if we can get the SmPL grammar expressed for it and then have
> developers / maintainers regularly doing:
>
> make coccicheck MODE=patch M=path > path-cocci.patch
>
> Unfortunately right now MODE=patch takes about 3 1/2 minutes for
> ath9k, MODE=org takes ~10 minutes for ath9k (17 minutes for all of
> ath/), and MODE=context takes ~8 minutes on ath9k -- I do believe its
> a bit unreasonable to expect patch submitters to use this, but
> certainly great practice. Some of the time differences on the reports
> can be explained by the fact that some SmPL will only be used for some
> modes.
>
> Even though it takes a while right now it'd be great practice to use
> coccicheck to prevent these type of changes from going in again,
> things that checkpatch.pl won't be able to catch.
As far as I can tell, it's basically not possible for cocci to
do this conversion.
There are other memcpy/memset uses with size_t = 6 where it's
not a MAC address being copied.
The call site has to be inspected to determine if the copied
to address is used as a MAC address or not.
Maybe if there was some typedef like:
typedef u8 eth_addr_t[ETH_ALEN];
There are a few of these already in use:
$ git grep "typedef.*\[" | grep -P "\[\s*(?:ETH_ALEN|6)\s*\]"
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h:typedef u8 bnx2x_mac_addr_t[ETH_ALEN];
drivers/net/ethernet/brocade/bna/cna.h:typedef struct mac { u8 mac[ETH_ALEN]; } mac_t;
drivers/net/ethernet/via/via-velocity.h:typedef u8 MCAM_ADDR[ETH_ALEN];
drivers/net/usb/kaweth.c:typedef __u8 eth_addr_t[6];
drivers/net/wireless/ray_cs.c:typedef u_char mac_addr[ETH_ALEN]; /* Hardware address */
drivers/staging/vt6655/wmgr.h:typedef unsigned char NDIS_802_11_MAC_ADDRESS[6];
drivers/staging/vt6656/wmgr.h:typedef u8 NDIS_802_11_MAC_ADDRESS[ETH_ALEN];
^ permalink raw reply
* Re: [PATCH net-next] net:drivers/net: Miscellaneous conversions to ETH_ALEN
From: Luis R. Rodriguez @ 2013-10-02 17:44 UTC (permalink / raw)
To: Joe Perches, Julia Lawall; +Cc: Kalle Valo, netdev, ath10k
In-Reply-To: <1380696054.2081.35.camel@joe-AO722>
On Tue, Oct 1, 2013 at 11:40 PM, Joe Perches <joe@perches.com> wrote:
> Please include netdev. (cc'd)
>
>> Joe Perches <joe@perches.com> writes:
>>
>> > Convert the memset/memcpy uses of 6 to ETH_ALEN
>> > where appropriate.
>
>> > Signed-off-by: Joe Perches <joe@perches.com>
I think these sorts of patches are good -- but once applied it'd be
good if we can get the SmPL grammar expressed for it and then have
developers / maintainers regularly doing:
make coccicheck MODE=patch M=path > path-cocci.patch
Unfortunately right now MODE=patch takes about 3 1/2 minutes for
ath9k, MODE=org takes ~10 minutes for ath9k (17 minutes for all of
ath/), and MODE=context takes ~8 minutes on ath9k -- I do believe its
a bit unreasonable to expect patch submitters to use this, but
certainly great practice. Some of the time differences on the reports
can be explained by the fact that some SmPL will only be used for some
modes.
Even though it takes a while right now it'd be great practice to use
coccicheck to prevent these type of changes from going in again,
things that checkpatch.pl won't be able to catch.
Luis
^ permalink raw reply
* Re: [net-next 2/3] udp: Add udp early demux
From: Shawn Bohrer @ 2013-10-02 17:34 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, tomk, netdev
In-Reply-To: <1380660769.19002.40.camel@edumazet-glaptop.roam.corp.google.com>
On Tue, Oct 01, 2013 at 01:52:49PM -0700, Eric Dumazet wrote:
> On Tue, 2013-10-01 at 14:33 -0500, Shawn Bohrer wrote:
> > The removal of the routing cache introduced a performance regression for
> > some UDP workloads since a dst lookup must be done for each packet.
> > This change caches the dst per socket in a similar manner to what we do
> > for TCP by implementing early_demux.
> >
> > For UDP multicast we can only cache the dst if there is only one
> > receiving socket on the host. Since caching only works when there is
> > one receiving socket we do the multicast socket lookup using RCU.
>
> For unicast, we should find a matching socket for early demux only if
> this is a connected socket.
>
> Otherwise, forwarding setups will break.
>
> You probably need to add a minimum score to __udp4_lib_lookup()
Perhaps I'm missing something but I don't think a minimum score would
work because compute_score() and compute_score2() have several ways of
returning a score of lets say 4 and I don't think they all mean the
socket is connected. Why not just test the socket returned by
__udp4_lib_lookup() to see if it is connected in
udp_v4_early_demux()? Something like:
sk = __udp4_lib_lookup(net, iph->saddr, uh->source,
iph->daddr, uh->dest, dif,
&udp_table);
/* Only demux connected sockets or forwarding setups will break */
if (sk && !inet_sk(sk)->inet_daddr)
return;
--
Shawn
--
---------------------------------------------------------------
This email, along with any attachments, is confidential. If you
believe you received this message in error, please contact the
sender immediately and delete all copies of the message.
Thank you.
^ permalink raw reply
* Re: [PATCH RFC] random: introduce get_random_bytes_busy_wait_initialized
From: Hannes Frederic Sowa @ 2013-10-02 17:18 UTC (permalink / raw)
To: Theodore Ts'o, Eric Dumazet, Tom Herbert, davem, netdev,
jesse.brandeburg, linux-kernel
In-Reply-To: <20131002151018.GA31579@thunk.org>
Hi!
Thanks for looking into this!
On Wed, Oct 02, 2013 at 11:10:18AM -0400, Theodore Ts'o wrote:
> On Wed, Sep 25, 2013 at 11:00:34AM +0200, Hannes Frederic Sowa wrote:
> > [PATCH RFC] random: introduce get_random_bytes_busy_wait_initialized
> >
> > We want to use good entropy for initializing the secret keys used for
> > hashing in the core network stack. So busy wait before extracting random
> > data until the nonblocking_pool is initialized.
> >
> > Further entropy is also gathered by interrupts, so we are guaranteed to
> > make progress here.
>
> One thing that makes me a bit worried is that on certain
> architectures, it may take quite a while before we get enough entropy
> such that the non-blocking pool gets initialized.
Yes, I understand. My main concern is that we would feed instruction
addresses of limited randomness into the entropy pool while busy looping
until the pool is initialized on uni-processor machines. It would only be
helpful on multiprocessor machines I guess.
So, I am currently a bit skeptic if this change does improve things and
have to think about this again.
> Speaking more generally, there are many different reasons why
> different parts of the kernel needs randomness. I've found a number
> of places (mostly in various file systems so far because I know that
> subsystem better) because we are trying to use a random number
> generator with a higher level of guarantees than what was really
> required.
>
> What's not completely clear to me is what's the potential danger if
> build_ehash_secret() is initialized with a value that might be known
> to an adversary. I'll note that inet_ehash_secret() is a 32-bit uint.
> A 32-bit number isn't all that hard for an adversary to brute force.
> If the answer is there's now oracle that can be used so an adversary
> can tell whether or not they have correctly figured out the ehash
> secret, then it's not that clear that it's worth blocking until the
> urandom pool has 128 bits of entropy, when ehash_secret is only a
> 32-bit value.
It may be possible to find multicollisions if the hash is known which could
lead to a DoS against the hash table if the lookups become linear (and someone
finds a fast way to do this for jash in this case, also depending if one
hashes variable or static size data). So this is minor issue currently.
But this is not my main concern:
We currently initialize the syncookie secrets pretty early on boot-up:
http://lxr.free-electrons.com/source/net/ipv4/syncookies.c#L31
Because of this problem I came up with another solution. My plan is to
introduce a macro 'net_get_random_once' which could be used to initialize
secret keys used for hashing as late as possible:
http://patchwork.ozlabs.org/patch/278308/
David Miller suggested that I should use static_keys to reduce overhead in the
fast-path and I am currently testing this change. I'll send it for review
maybe tomorrow. Otherwise I have to come up with another solution, maybe only
specific for syncookies in the beginning.
> Speaking even more generally, any time you have subsystems grabbing
> random entropy very shortly after boot, especially if it's less than
> 64 bits, it's really good idea of the secret gets periodically
> rekeyed. I understand why that may be hard in this case, since it
> would require rehashing all of the currently open sockets, and maybe
> in this case the security requirements are such that it's not really
> necessary. But it's something we should definitely keep in mind for
> situations where we are generating random session keys for CIFS,
> ipsec, etc.
I agree. I will look if this is easily possible for secure_seq and
syncookies but depending on the data structure and its size it is a much
harder thing to do. I wanted to try the low-hanging fruits first. ;)
Thanks,
Hannes
^ permalink raw reply
* The check of upper MTU limit when changing it in ip6 gre tunnel seems incorrect.
From: Oussama Ghorbel @ 2013-10-02 16:44 UTC (permalink / raw)
To: netdev
The check of upper MTU limit when changing it in ip6 gre tunnel seems incorrect.
The function in question is:
static int ip6gre_tunnel_change_mtu(struct net_device *dev, int new_mtu)
{
struct ip6_tnl *tunnel = netdev_priv(dev);
if (new_mtu < 68 ||
new_mtu > 0xFFF8 - dev->hard_header_len - tunnel->hlen)
return -EINVAL;
dev->mtu = new_mtu;
return 0;
}
However the dev->hard_header_len and tunnel->hlen are initialized in
the following way in ip6gre_tnl_link_config():
int addend = sizeof(struct ipv6hdr) + 4;
...
dev->hard_header_len = rt->dst.dev->hard_header_len + addend;
...
t->hlen = addend; // t is ip6_tnl pointer
As you see the information t->hlen is already included in
dev->hard_header_len, so why calculate it twice?
Thanks
^ permalink raw reply
* Neterion and UFO handling [was: Re: [PATCH] ipv6: udp packets following an UFO enqueued packet need also be handled by UFO]
From: Hannes Frederic Sowa @ 2013-10-02 16:27 UTC (permalink / raw)
To: jdmason
Cc: Jiri Pirko, netdev, yoshfuji, davem, kuznet, jmorris, kaber,
herbert, eric.dumazet
In-Reply-To: <1380726867.19002.104.camel@edumazet-glaptop.roam.corp.google.com>
Hi!
I have a question regarding UFO and the neterion driver, which as the only one
advertises hardware UFO support:
The patch discusses in this thread
http://thread.gmane.org/gmane.linux.network/284348/focus=285405 could change
some semantics how packets are constructed before submitted to the driver.
We currently guarantee that we have the MAC/IP/UDP header in skb->data and the
payload is attached in the skb's frags. With the changes discussed in this
thread it is possible that we also append to skb->data some amount of data
which is not targeted for the header. From reading the driver sources it seems
the hardware interprets the skb->data to skb_headlen as the header, so we
could include some data in the fragments more than once.
Do you think this change is safe? Otherwise I would suggest that the UFO
capability is switched off until the driver signals the hardware the start and
end of the headers correctly?
I left the mail below intact which points to the specific place in s2io.c
where I think the problem is.
On Wed, Oct 02, 2013 at 08:14:27AM -0700, Eric Dumazet wrote:
> On Wed, 2013-10-02 at 15:03 +0200, Hannes Frederic Sowa wrote:
> > On Wed, Oct 02, 2013 at 02:12:07PM +0200, Hannes Frederic Sowa wrote:
> > > Hi Eric!
> > >
> > > On Wed, Oct 02, 2013 at 03:41:28AM -0700, Eric Dumazet wrote:
> > > > On Wed, 2013-10-02 at 10:58 +0200, Jiri Pirko wrote:
> > > > > Wed, Oct 02, 2013 at 01:25:34AM CEST, hannes@stressinduktion.org wrote:
> > > > > >- if (((length > mtu) || (skb && skb_is_gso(skb))) &&
> > > > > >+ if (((length > mtu) || (skb && skb_has_frags(skb))) &&
> > > >
> > > > >
> > > > > This seems correct to me. sk_is_gso would work as well is you apply my
> > > > > patch "[patch net] ip6_output: do skb ufo init for peeked non ufo skb as
> > > > > well" which does the setting of gso_size.
> > > >
> > > > Well, skb having frags or not should not be a concern :
> > > > Thats an allocation choice (lets say to avoid high order allocations).
> > > >
> > > > Setting gso_size is probably better.
> > >
> > > e89e9cf539a28df7d0eb1d0a545368e9920b34ac ("[IPv4/IPv6]: UFO Scatter-gather
> > > approach") states:
> > >
> > > "
> > > skb->data will contain MAC/IP/UDP header and skb_shinfo(skb)->frags[]
> > > contains the data payload. The skb->ip_summed will be set to CHECKSUM_HW
> > > indicating that hardware has to do checksum calculation. Hardware should
> > > compute the UDP checksum of complete datagram and also ip header checksum of
> > > each fragmented IP packet.
> > > "
> > >
> > > This is the reason why I tried not to update the gso_size. If it is ok, I am
> > > fine with that.
> >
> > Especially, drivers/net/ethernet/neterion/s2io.c states that the first dma
> > mapping (skb->data with skb_headlen, which is fine) is used as the inband
> > header:
> >
> > if (offload_type == SKB_GSO_UDP)
> > frg_cnt++; /* as Txd0 was used for inband header */
> >
> > That is my only other hint that we maybe should not update gso_size and
> > gso_type. I guess software fallback does not have this problem, but I won't
> > have time to check until this evening.
> >
> > I am really not sure if just setting gso_size does not break neterion UFO
> > offloading. :/
>
> Well, just ask Jon Mason to double check ;)
>
> I think the commit intent was to set gso_size :
>
> skb_shinfo(skb)->ufo_size will indicate the length of data part in each IP
> fragment going out of the adapter after IP fragmentation by hardware.
>
> The fact that it states "skb->data will contain MAC/IP/UDP header and
> skb_shinfo(skb)->frags[] contains the data payload." seems irrelevant.
>
> If Neterion driver mandates that skb->head *only* contains the
> MAC/IP/UDP header, that should be handled in the driver itself.
Thanks Eric for clearing this up.
I really thought it would be the common pattern for UFO to have only headers
in skb->data, so I didn't bother to ask in the first place.
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH] ipv6: udp packets following an UFO enqueued packet need also be handled by UFO
From: Eric Dumazet @ 2013-10-02 15:14 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Jiri Pirko, netdev, yoshfuji, davem, kuznet, jmorris, kaber,
herbert
In-Reply-To: <20131002130329.GP10771@order.stressinduktion.org>
On Wed, 2013-10-02 at 15:03 +0200, Hannes Frederic Sowa wrote:
> On Wed, Oct 02, 2013 at 02:12:07PM +0200, Hannes Frederic Sowa wrote:
> > Hi Eric!
> >
> > On Wed, Oct 02, 2013 at 03:41:28AM -0700, Eric Dumazet wrote:
> > > On Wed, 2013-10-02 at 10:58 +0200, Jiri Pirko wrote:
> > > > Wed, Oct 02, 2013 at 01:25:34AM CEST, hannes@stressinduktion.org wrote:
> > > > >- if (((length > mtu) || (skb && skb_is_gso(skb))) &&
> > > > >+ if (((length > mtu) || (skb && skb_has_frags(skb))) &&
> > >
> > > >
> > > > This seems correct to me. sk_is_gso would work as well is you apply my
> > > > patch "[patch net] ip6_output: do skb ufo init for peeked non ufo skb as
> > > > well" which does the setting of gso_size.
> > >
> > > Well, skb having frags or not should not be a concern :
> > > Thats an allocation choice (lets say to avoid high order allocations).
> > >
> > > Setting gso_size is probably better.
> >
> > e89e9cf539a28df7d0eb1d0a545368e9920b34ac ("[IPv4/IPv6]: UFO Scatter-gather
> > approach") states:
> >
> > "
> > skb->data will contain MAC/IP/UDP header and skb_shinfo(skb)->frags[]
> > contains the data payload. The skb->ip_summed will be set to CHECKSUM_HW
> > indicating that hardware has to do checksum calculation. Hardware should
> > compute the UDP checksum of complete datagram and also ip header checksum of
> > each fragmented IP packet.
> > "
> >
> > This is the reason why I tried not to update the gso_size. If it is ok, I am
> > fine with that.
>
> Especially, drivers/net/ethernet/neterion/s2io.c states that the first dma
> mapping (skb->data with skb_headlen, which is fine) is used as the inband
> header:
>
> if (offload_type == SKB_GSO_UDP)
> frg_cnt++; /* as Txd0 was used for inband header */
>
> That is my only other hint that we maybe should not update gso_size and
> gso_type. I guess software fallback does not have this problem, but I won't
> have time to check until this evening.
>
> I am really not sure if just setting gso_size does not break neterion UFO
> offloading. :/
Well, just ask Jon Mason to double check ;)
I think the commit intent was to set gso_size :
skb_shinfo(skb)->ufo_size will indicate the length of data part in each IP
fragment going out of the adapter after IP fragmentation by hardware.
The fact that it states "skb->data will contain MAC/IP/UDP header and
skb_shinfo(skb)->frags[] contains the data payload." seems irrelevant.
If Neterion driver mandates that skb->head *only* contains the
MAC/IP/UDP header, that should be handled in the driver itself.
^ permalink raw reply
* Re: [PATCH RFC] random: introduce get_random_bytes_busy_wait_initialized
From: Theodore Ts'o @ 2013-10-02 15:10 UTC (permalink / raw)
To: Eric Dumazet, Tom Herbert, davem, netdev, jesse.brandeburg,
linux-kernel
In-Reply-To: <20130925090034.GC4904@order.stressinduktion.org>
On Wed, Sep 25, 2013 at 11:00:34AM +0200, Hannes Frederic Sowa wrote:
> [PATCH RFC] random: introduce get_random_bytes_busy_wait_initialized
>
> We want to use good entropy for initializing the secret keys used for
> hashing in the core network stack. So busy wait before extracting random
> data until the nonblocking_pool is initialized.
>
> Further entropy is also gathered by interrupts, so we are guaranteed to
> make progress here.
One thing that makes me a bit worried is that on certain
architectures, it may take quite a while before we get enough entropy
such that the non-blocking pool gets initialized.
Speaking more generally, there are many different reasons why
different parts of the kernel needs randomness. I've found a number
of places (mostly in various file systems so far because I know that
subsystem better) because we are trying to use a random number
generator with a higher level of guarantees than what was really
required.
What's not completely clear to me is what's the potential danger if
build_ehash_secret() is initialized with a value that might be known
to an adversary. I'll note that inet_ehash_secret() is a 32-bit uint.
A 32-bit number isn't all that hard for an adversary to brute force.
If the answer is there's now oracle that can be used so an adversary
can tell whether or not they have correctly figured out the ehash
secret, then it's not that clear that it's worth blocking until the
urandom pool has 128 bits of entropy, when ehash_secret is only a
32-bit value.
Speaking even more generally, any time you have subsystems grabbing
random entropy very shortly after boot, especially if it's less than
64 bits, it's really good idea of the secret gets periodically
rekeyed. I understand why that may be hard in this case, since it
would require rehashing all of the currently open sockets, and maybe
in this case the security requirements are such that it's not really
necessary. But it's something we should definitely keep in mind for
situations where we are generating random session keys for CIFS,
ipsec, etc.
Regards,
- Ted
^ permalink raw reply
* [PATCH net-next] 3com: Fix drivers/net/ethernet/3com/Kconfig references to PCMCIA and 3c515
From: Matthew Whitehead @ 2013-10-02 15:08 UTC (permalink / raw)
To: netdev; +Cc: Matthew Whitehead
The Vortex driver works with PCI and Cardbus devices, not PCMCIA.
There never was an EISA 3c515 card, only ISA, so remove that option.
Signed-off-by: Matthew Whitehead <tedheadster@gmail.com>
---
drivers/net/ethernet/3com/Kconfig | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/3com/Kconfig b/drivers/net/ethernet/3com/Kconfig
index f00c763..65b735d 100644
--- a/drivers/net/ethernet/3com/Kconfig
+++ b/drivers/net/ethernet/3com/Kconfig
@@ -35,7 +35,7 @@ config EL3
config 3C515
tristate "3c515 ISA \"Fast EtherLink\""
- depends on (ISA || EISA) && ISA_DMA_API
+ depends on ISA && ISA_DMA_API
---help---
If you have a 3Com ISA EtherLink XL "Corkscrew" 3c515 Fast Ethernet
network card, say Y and read the Ethernet-HOWTO, available from
@@ -70,7 +70,7 @@ config VORTEX
select MII
---help---
This option enables driver support for a large number of 10Mbps and
- 10/100Mbps EISA, PCI and PCMCIA 3Com network cards:
+ 10/100Mbps EISA, PCI and Cardbus 3Com network cards:
"Vortex" (Fast EtherLink 3c590/3c592/3c595/3c597) EISA and PCI
"Boomerang" (EtherLink XL 3c900 or 3c905) PCI
--
1.7.2.5
^ permalink raw reply related
* RE: [PATCH net-next] tcp: shrink tcp6_timewait_sock by one cache line
From: Eric Dumazet @ 2013-10-02 14:57 UTC (permalink / raw)
To: David Laight; +Cc: David Miller, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B736F@saturn3.aculab.com>
On Wed, 2013-10-02 at 13:08 +0100, David Laight wrote:
> > - tmo = tw->tw_ttd - jiffies;
> > + tmo = tw->tw_ttd - (u32)jiffies;
>
> Do you need any of these (u32) casts?
> The compiler will almost certainly use 32bit arithmetic (on 32bit systems at least)
> because the 'as if' rule lets if use the smaller type.
I wanted to clearly show the intent of the code.
Some compilers might warnings here.
>
> > + tw->tw_ttd = (u32)(jiffies + (slot << INET_TWDR_RECYCLE_TICK));
>
> If that (u32) cast is needed in order to avoid 64bit maths, it is in the wrong place.
I want to make sure no compiler will complain for potential losses of
precision.
Note we have :
include/net/tcp.h:691:#define tcp_time_stamp ((__u32)(jiffies))
But this could change if we want 100us resolution for TCP timestamps at
one point.
In this code, we want HZ units, but truncated to 32bits.
Adding a
#define jiffies32 ((u32)jiffies)
might do the trick, but lot of pain for such a trivial patch.
^ permalink raw reply
* Re: [RFC PATCH 0/2] net: alternate proposal for using macvlans with forwarding acceleration
From: Neil Horman @ 2013-10-02 13:28 UTC (permalink / raw)
To: John Fastabend; +Cc: netdev
In-Reply-To: <524BBDC1.6040000@gmail.com>
On Tue, Oct 01, 2013 at 11:31:29PM -0700, John Fastabend wrote:
> On 09/25/2013 01:16 PM, Neil Horman wrote:
> >John, et al. -
> > As promised, heres my (very rough) first pass at an alternate propsal for
> >what you're trying to do with virtual station interfaces here. Its completely
> >untested, but it builds, and I'll be trying to run it over the next few days
> >(though I'm sure I got part of the hardware manipulation wrong). I wanted to
> >post it early though so you could get a look at it to see what you did and
> >didn't like about it. Some notes:
>
> Sorry for the delay. I like the idea one nice win here is my macvlan
> kvm setup would use the offloads without having to reconfigure.
>
Thats ok, I've been swamped, so this has gone slower than I had hoped for me.
> >
> >1) As discussed, the major effort here is to tie in macvlans with l2 forwarding
> >acceleration, rather than creating a new vsi link type. That should make
> >management easier for admins (be it via ovs or some other mechanism). It
> >basically exposes a bit less to the user, which I think is good.
> >
>
> The trick here is the offload path may be functionally different from
> the non-offload path. The user needs some visibility into this. For
> example any qdiscs running on the lowerdev will not be visible from the
> accelerated path.
>
Actually, given your suggestion in your other note (using dev_queue_xmit), I
think this will be handled, in that we will now pass accelerated traffic through
any lowerdev attached qdiscs.
> When a new link type was being used I was able to convince myself that
> it was a property of the link type. But if we reuse macvlan I think we
> need some way to either automatically disable the offload path when this
> occurs or provide the user visibility. Maybe a feature flag and a
> netif_can_hw_offload() routine is needed?
>
As noted above, I think we can make macvlans work identially with and without
acceleration by reusing the xmit path, and keying off data (what I've done is
attach a pointer to acceleration data in the skb so the common ndo_start_xmit
path in the driver can differentiate accelerated from non-accelerated skbs).
That will allow everything to function the same way. I do think though that a
feature flag just to provide some visibility to the admin is warranted.
> >2) I've separated out the l2 forwarding acceleration operations from the
> >net_device_operations structure. I'm not sure I like that yet, but I'm kind on
> >leaning that way. Since a limited set of hardare supports forwarding
> >acceleration, it makes for a nice easy way to group functionality without
> >polluting the net_device_operations structure. It also lets us group simmilar
> >functions together nicely (I can see a future l3_accel_ops structure if we can
> >do l3 flows in hardware). Anywho, its a divergence from what we've been doing
> >so I thought I would call attention to it.
> >
> >3) I've included a l2_accel_xmit method in the accel_ops structure for fast path
> >forwarding, but I'm not sure I like that. It seems we should be able to use
> >ndo_start_xmit and key off some data to recognize that we should be doing
> >hardware forwarding. I'm not quite sure how to do that yet though. Something
> >to think about.
>
> Without a specific xmit routine though you will be adding operations in
> the common case for a special case. Having a new op fixes this.
>
Agreed, and thats something to consider. I'm not sure if separating the special
case to its own operation is a net win however, given the behavioral differences
that arise (the qdisc issue above). My next version will use a common xmit
path. We can look it over then for comparison and weight the pros and cons.
> >
> >4) I've borrowed heavily from your vsi work of course just to get this building.
> >I think theres probbaly alot of consolidation that can be done in the code that
> >I added to ixgbe_main.c to make it smaller. Again, I just wanted to post this
> >so you could speak up if you though this was all crap before I wen't too far
> >down the rabbit hole.
>
> There was some consolidation needed in my original RFC as well. I can
> help clean some of this stuff up if you want.
>
That would be greatly appreciated. Thanks. Next version should be available
in the next few days. I'll get it to you asap.
Best
Neil
> .John
>
> --
> John Fastabend Intel Corporation
>
^ permalink raw reply
* Confirm your webmail Identity
From: Webmail Helpdesk Support @ 2013-10-02 13:16 UTC (permalink / raw)
Confirm your webmail Identity
You have reached the quota limit of email. This will not be able to
send or receive incomming mails,increases mailbox size until a new
message. Click the below link to update your account.
itservice-upgrade.phpforms.net/f/webadminn
Regards
Webmail HelpDesk Support
Copyright © 2013,Webmail, Technical Support Team, All Rights Reserved
^ permalink raw reply
* Re: [PATCH] ipv6: udp packets following an UFO enqueued packet need also be handled by UFO
From: Hannes Frederic Sowa @ 2013-10-02 13:03 UTC (permalink / raw)
To: Eric Dumazet, Jiri Pirko, netdev, yoshfuji, davem, kuznet,
jmorris, kaber, herbert
In-Reply-To: <20131002121207.GO10771@order.stressinduktion.org>
On Wed, Oct 02, 2013 at 02:12:07PM +0200, Hannes Frederic Sowa wrote:
> Hi Eric!
>
> On Wed, Oct 02, 2013 at 03:41:28AM -0700, Eric Dumazet wrote:
> > On Wed, 2013-10-02 at 10:58 +0200, Jiri Pirko wrote:
> > > Wed, Oct 02, 2013 at 01:25:34AM CEST, hannes@stressinduktion.org wrote:
> > > >- if (((length > mtu) || (skb && skb_is_gso(skb))) &&
> > > >+ if (((length > mtu) || (skb && skb_has_frags(skb))) &&
> >
> > >
> > > This seems correct to me. sk_is_gso would work as well is you apply my
> > > patch "[patch net] ip6_output: do skb ufo init for peeked non ufo skb as
> > > well" which does the setting of gso_size.
> >
> > Well, skb having frags or not should not be a concern :
> > Thats an allocation choice (lets say to avoid high order allocations).
> >
> > Setting gso_size is probably better.
>
> e89e9cf539a28df7d0eb1d0a545368e9920b34ac ("[IPv4/IPv6]: UFO Scatter-gather
> approach") states:
>
> "
> skb->data will contain MAC/IP/UDP header and skb_shinfo(skb)->frags[]
> contains the data payload. The skb->ip_summed will be set to CHECKSUM_HW
> indicating that hardware has to do checksum calculation. Hardware should
> compute the UDP checksum of complete datagram and also ip header checksum of
> each fragmented IP packet.
> "
>
> This is the reason why I tried not to update the gso_size. If it is ok, I am
> fine with that.
Especially, drivers/net/ethernet/neterion/s2io.c states that the first dma
mapping (skb->data with skb_headlen, which is fine) is used as the inband
header:
if (offload_type == SKB_GSO_UDP)
frg_cnt++; /* as Txd0 was used for inband header */
That is my only other hint that we maybe should not update gso_size and
gso_type. I guess software fallback does not have this problem, but I won't
have time to check until this evening.
I am really not sure if just setting gso_size does not break neterion UFO
offloading. :/
Greetings,
Hannes
^ 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