* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Ben Hutchings @ 2011-05-27 15:45 UTC (permalink / raw)
To: Michał Mirosław; +Cc: David Miller, netdev
In-Reply-To: <20110527152809.GA7620@rere.qmqm.pl>
On Fri, 2011-05-27 at 17:28 +0200, Michał Mirosław wrote:
> On Fri, May 27, 2011 at 03:13:46PM +0100, Ben Hutchings wrote:
> > On Tue, 2011-05-24 at 23:59 +0200, Michał Mirosław wrote:
> > > On Tue, May 24, 2011 at 03:39:30PM -0400, David Miller wrote:
> > > > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > > Date: Tue, 24 May 2011 11:14:37 +0200
> > > > > On Thu, May 19, 2011 at 12:03:31PM +0200, Michał Mirosław wrote:
> > > > >> On Mon, May 16, 2011 at 02:09:58PM -0400, David Miller wrote:
> > > > >> > You guys really need to sort this out properly.
> > > > >> > Please resubmit whatever final solution is agreed upon.
> > > > >> I noticed that v2.6.39 was tagged today. We should definitely remove
> > > > >> NETIF_F_COMPAT so it won't bite us in the future. The other patch that
> > > > >> fixes ethtool_ops->set_flags compatibility is a bugfix, so it should go
> > > > >> in - if we decide that the SFEATURES compatibility should be removed
> > > > >> it won't matter.
> [...]
> > > We could just wait for 2.6.40 and pretend this code part never existed. ;-)
> > I think I will make ethtool check for a minimum kernel version of 2.6.40
> > before using ETHTOOL_{G,S}FEATURES.
>
> > > I'll rebase the first patch tomorrow. Without it the compatibility in
> > > ETHTOOL_SFEATURES for non-converted drivers is busted wrt set_flags.
> > This is an improvement, but I still think the fallback is fundamentally
> > broken - there's no good way for userland to tell what (if anything)
> > went wrong when the return value has ETHTOOL_F_COMPAT set.
>
> The same situation happens with ETHTOOL_F_WISH (userspace needs to reread
> the features to find out what happened) and with old ETHTOOL_S{TSO,SG,...}
> (those return success if any of the features in the group changes and also
> posibly disable other features when one is disabled). This wasn't really
> checked before.
>
> Ben, I think I commented on your proposal of the userspace part, but I might
> have missed some of your arguments about mine. Let's sum those up:
>
> Your version:
> - reimplements ETHTOOL_Sxx via ETHTOOL_SFEATURES in userspace for kernels
> supporting the latter
No, it implements 'ethtool -K' using ETHTOOL_SFEATURES. Maybe you
consider the ethtool utility to be a thin wrapper over the ethtool API,
but that is not my intent.
> (note: ETHTOOL_S{SG,...} are not ever going away)
> - causes NETIF_F_* to be an ABI
If feature flag numbers are not stable then what is the point of
/sys/class/net/<name>/features? Also, I'm not aware that features have
ever been renumbered in the past.
I think ethtool should maintain a feature bitmask rather than the
separate flags it currently does, and I previously attempted this using
a private set of flags. Shortly afterward that, you proposed to
introduce the new features interfaces, and it seemed to me to make sense
to use the net device feature flags in ethtool.
David, do you think feature flag numbers should be considered a
userspace (i.e. stable) ABI or not?
> - does not support new features
Not immediately. I intend to do that afterward.
> My version:
> - implements only new features via ETHTOOL_SFEATURES (old calls are still used)
> - makes feature names an ABI (for scripts actually, not the tool)
> - supports any new features kernel reports without code changes
Right. I definitely should incorporate your code for looking up
features by string.
> Both versions are rough at the edges, but working. Both assume that no
> behaviour changes are to be made for old '-K' options.
No, my changes are intended to enhance the old options.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] caif: Fix compile warning in caif_serial.c
From: Randy Dunlap @ 2011-05-27 15:37 UTC (permalink / raw)
To: Sjur Brændeland; +Cc: Stephen Rothwell, linux-next, balbi, netdev
In-Reply-To: <1306483755-3588-1-git-send-email-sjur.brandeland@stericsson.com>
On 05/27/11 01:09, Sjur Brændeland wrote:
> Fix the compile warning introduced by the patch:
> "tty: make receive_buf() return the amout of bytes received"
>
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
> ---
> Note: Fixes compile issue in linux-next (no issue in net-2.6).
Acked-by: Randy Dunlap <randy.dunlap@oracle.com>
Thanks much, Sjur.
> drivers/net/caif/caif_serial.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> index 73c7e03..751ebbd 100644
> --- a/drivers/net/caif/caif_serial.c
> +++ b/drivers/net/caif/caif_serial.c
> @@ -191,7 +191,7 @@ static unsigned int ldisc_receive(struct tty_struct *tty,
> dev_info(&ser->dev->dev,
> "Bytes received before initial transmission -"
> "bytes discarded.\n");
> - return;
> + return count;
> }
>
> BUG_ON(ser->dev == NULL);
> @@ -199,7 +199,7 @@ static unsigned int ldisc_receive(struct tty_struct *tty,
> /* Get a suitable caif packet and copy in data. */
> skb = netdev_alloc_skb(ser->dev, count+1);
> if (skb == NULL)
> - return;
> + return 0;
> p = skb_put(skb, count);
> memcpy(p, data, count);
>
--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
^ permalink raw reply
* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Michał Mirosław @ 2011-05-27 15:28 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, netdev
In-Reply-To: <1306505626.2759.4.camel@bwh-desktop>
On Fri, May 27, 2011 at 03:13:46PM +0100, Ben Hutchings wrote:
> On Tue, 2011-05-24 at 23:59 +0200, Michał Mirosław wrote:
> > On Tue, May 24, 2011 at 03:39:30PM -0400, David Miller wrote:
> > > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > > Date: Tue, 24 May 2011 11:14:37 +0200
> > > > On Thu, May 19, 2011 at 12:03:31PM +0200, Michał Mirosław wrote:
> > > >> On Mon, May 16, 2011 at 02:09:58PM -0400, David Miller wrote:
> > > >> > You guys really need to sort this out properly.
> > > >> > Please resubmit whatever final solution is agreed upon.
> > > >> I noticed that v2.6.39 was tagged today. We should definitely remove
> > > >> NETIF_F_COMPAT so it won't bite us in the future. The other patch that
> > > >> fixes ethtool_ops->set_flags compatibility is a bugfix, so it should go
> > > >> in - if we decide that the SFEATURES compatibility should be removed
> > > >> it won't matter.
[...]
> > We could just wait for 2.6.40 and pretend this code part never existed. ;-)
> I think I will make ethtool check for a minimum kernel version of 2.6.40
> before using ETHTOOL_{G,S}FEATURES.
> > I'll rebase the first patch tomorrow. Without it the compatibility in
> > ETHTOOL_SFEATURES for non-converted drivers is busted wrt set_flags.
> This is an improvement, but I still think the fallback is fundamentally
> broken - there's no good way for userland to tell what (if anything)
> went wrong when the return value has ETHTOOL_F_COMPAT set.
The same situation happens with ETHTOOL_F_WISH (userspace needs to reread
the features to find out what happened) and with old ETHTOOL_S{TSO,SG,...}
(those return success if any of the features in the group changes and also
posibly disable other features when one is disabled). This wasn't really
checked before.
Ben, I think I commented on your proposal of the userspace part, but I might
have missed some of your arguments about mine. Let's sum those up:
Your version:
- reimplements ETHTOOL_Sxx via ETHTOOL_SFEATURES in userspace for kernels
supporting the latter (note: ETHTOOL_S{SG,...} are not ever going away)
- causes NETIF_F_* to be an ABI
- does not support new features
My version:
- implements only new features via ETHTOOL_SFEATURES (old calls are still used)
- makes feature names an ABI (for scripts actually, not the tool)
- supports any new features kernel reports without code changes
Both versions are rough at the edges, but working. Both assume that no
behaviour changes are to be made for old '-K' options.
Best Regards,
Michał Mirosław
^ permalink raw reply
* [PATCH] netfilter: nf_conntrack: remove one synchronize_net()
From: Eric Dumazet @ 2011-05-27 15:15 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Netfilter Development Mailinglist, netdev
No point to wait a rcu grace period before the unregisters.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index 5a03c02..b09bfeb 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -435,7 +435,6 @@ static int __init nf_conntrack_l3proto_ipv4_init(void)
static void __exit nf_conntrack_l3proto_ipv4_fini(void)
{
- synchronize_net();
#if defined(CONFIG_PROC_FS) && defined(CONFIG_NF_CONNTRACK_PROC_COMPAT)
nf_conntrack_ipv4_compat_fini();
#endif
^ permalink raw reply related
* Re: [PATCH v2] atm: expose ATM device index in sysfs
From: David Woodhouse @ 2011-05-27 15:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Dan Williams, netdev, Chas Williams
In-Reply-To: <1306508269.3445.0.camel@edumazet-laptop>
On Fri, 2011-05-27 at 16:57 +0200, Eric Dumazet wrote:
> Le vendredi 27 mai 2011 à 09:51 -0500, Dan Williams a écrit :
> > It's currently exposed only through /proc which, besides requiring
> > screen-scraping, doesn't allow userspace to distinguish between two
> > identical ATM adapters with different ATM indexes. The ATM device index
> > is required when using PPPoATM on a system with multiple ATM adapters.
> >
> > Signed-off-by: Dan Williams <dcbw@redhat.com>
> > ---
> >
>
> Reviewed-by: Eric Dumazet <eric.dumazet@gmail.com>
Tested-by: David Woodhouse <dwmw2@infradead.org>
Cc: stable@kernel.org
[root@solos atm]# grep ^ /sys/class/atm/*/atmindex
/sys/class/atm/solos-pci0/atmindex:0
/sys/class/atm/solos-pci1/atmindex:1
--
dwmw2
^ permalink raw reply
* Re: [PATCH v2] atm: expose ATM device index in sysfs
From: Eric Dumazet @ 2011-05-27 14:57 UTC (permalink / raw)
To: Dan Williams; +Cc: netdev, Chas Williams, David Woodhouse
In-Reply-To: <1306507915.22126.2.camel@dcbw.foobar.com>
Le vendredi 27 mai 2011 à 09:51 -0500, Dan Williams a écrit :
> It's currently exposed only through /proc which, besides requiring
> screen-scraping, doesn't allow userspace to distinguish between two
> identical ATM adapters with different ATM indexes. The ATM device index
> is required when using PPPoATM on a system with multiple ATM adapters.
>
> Signed-off-by: Dan Williams <dcbw@redhat.com>
> ---
>
Reviewed-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* [PATCH v2] atm: expose ATM device index in sysfs
From: Dan Williams @ 2011-05-27 14:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Chas Williams, David Woodhouse
In-Reply-To: <1306434959.3151.39.camel@dcbw.foobar.com>
It's currently exposed only through /proc which, besides requiring
screen-scraping, doesn't allow userspace to distinguish between two
identical ATM adapters with different ATM indexes. The ATM device index
is required when using PPPoATM on a system with multiple ATM adapters.
Signed-off-by: Dan Williams <dcbw@redhat.com>
---
diff --git a/net/atm/atm_sysfs.c b/net/atm/atm_sysfs.c
index f7fa67c..f49da58 100644
--- a/net/atm/atm_sysfs.c
+++ b/net/atm/atm_sysfs.c
@@ -59,6 +59,14 @@ static ssize_t show_atmaddress(struct device *cdev,
return pos - buf;
}
+static ssize_t show_atmindex(struct device *cdev,
+ struct device_attribute *attr, char *buf)
+{
+ struct atm_dev *adev = to_atm_dev(cdev);
+
+ return sprintf(buf, "%d\n", adev->number);
+}
+
static ssize_t show_carrier(struct device *cdev,
struct device_attribute *attr, char *buf)
{
@@ -99,6 +107,7 @@ static ssize_t show_link_rate(struct device *cdev,
static DEVICE_ATTR(address, S_IRUGO, show_address, NULL);
static DEVICE_ATTR(atmaddress, S_IRUGO, show_atmaddress, NULL);
+static DEVICE_ATTR(atmindex, S_IRUGO, show_atmindex, NULL);
static DEVICE_ATTR(carrier, S_IRUGO, show_carrier, NULL);
static DEVICE_ATTR(type, S_IRUGO, show_type, NULL);
static DEVICE_ATTR(link_rate, S_IRUGO, show_link_rate, NULL);
@@ -106,6 +115,7 @@ static DEVICE_ATTR(link_rate, S_IRUGO, show_link_rate, NULL);
static struct device_attribute *atm_attrs[] = {
&dev_attr_atmaddress,
&dev_attr_address,
+ &dev_attr_atmindex,
&dev_attr_carrier,
&dev_attr_type,
&dev_attr_link_rate,
^ permalink raw reply related
* Re: [PATCH] ethtool: ETHTOOL_SFEATURES: remove NETIF_F_COMPAT return
From: Ben Hutchings @ 2011-05-27 14:13 UTC (permalink / raw)
To: Michał Mirosław; +Cc: David Miller, netdev
In-Reply-To: <20110524215923.GA20138@rere.qmqm.pl>
On Tue, 2011-05-24 at 23:59 +0200, Michał Mirosław wrote:
> On Tue, May 24, 2011 at 03:39:30PM -0400, David Miller wrote:
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Date: Tue, 24 May 2011 11:14:37 +0200
> >
> > > On Thu, May 19, 2011 at 12:03:31PM +0200, Michał Mirosław wrote:
> > >> On Mon, May 16, 2011 at 02:09:58PM -0400, David Miller wrote:
> > >> > You guys really need to sort this out properly.
> > >> > Please resubmit whatever final solution is agreed upon.
> > >> I noticed that v2.6.39 was tagged today. We should definitely remove
> > >> NETIF_F_COMPAT so it won't bite us in the future. The other patch that
> > >> fixes ethtool_ops->set_flags compatibility is a bugfix, so it should go
> > >> in - if we decide that the SFEATURES compatibility should be removed
> > >> it won't matter.
> > >>
> > >> Ben, do you agree?
> > > Ping?
> > > http://patchwork.ozlabs.org/patch/95552/
> > > (this is a bugfix, so should go to stable)
> > >
> > > http://patchwork.ozlabs.org/patch/95753/
> > > (removes ETHTOOL_F_COMPAT; this we need to decide on)
> > You and Ben are still arguing over details.
> >
> > I want fresh versions of these patches (yes, both of them) once
> > all of the issues are resolved.
>
> We could just wait for 2.6.40 and pretend this code part never existed. ;-)
I think I will make ethtool check for a minimum kernel version of 2.6.40
before using ETHTOOL_{G,S}FEATURES.
> I'll rebase the first patch tomorrow. Without it the compatibility in
> ETHTOOL_SFEATURES for non-converted drivers is busted wrt set_flags.
This is an improvement, but I still think the fallback is fundamentally
broken - there's no good way for userland to tell what (if anything)
went wrong when the return value has ETHTOOL_F_COMPAT set.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.11
From: Pablo Neira Ayuso @ 2011-05-27 11:55 UTC (permalink / raw)
To: Arkadiusz Miskiewicz
Cc: shemminger, Netfilter Development Mailinglist, NetDev
In-Reply-To: <201105270952.44811.a.miskiewicz@gmail.com>
On 27/05/11 09:52, Arkadiusz Miskiewicz wrote:
> On Thursday 26 of May 2011, Patrick McHardy wrote:
>> Am 26.05.2011 18:53, schrieb Patrick McHardy:
>>> The netfilter coreteam presents:
>>> iptables version 1.4.10
>>
>> That's supposed to read 1.4.11 of course :)
>
> Too bad it breaks iproute2 build, hope to see fixed iproute2 release then
>
> gcc -D_GNU_SOURCE -O2 -Wstrict-prototypes -Wall -I../include -
> DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib/\" -DCONFIG_GACT -DCONFIG_GACT_PROB -
> DIPT_LIB_DIR=\"/usr/lib64/xtables\" -Wl,-export-dynamic -shared -fpic -o
> q_atm.so q_atm.c -latm
> gcc -D_GNU_SOURCE -O2 -Wstrict-prototypes -Wall -I../include -
> DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib/\" -DCONFIG_GACT -DCONFIG_GACT_PROB -
> DIPT_LIB_DIR=\"/usr/lib64/xtables\" -Wl,-export-dynamic -shared -fpic -o
> m_xt.so m_xt.c -lxtables
> m_xt.c: In function ‘parse_ipt’:
> m_xt.c:167:31: warning: passing argument 2 of ‘xtables_merge_options’ discards
> ‘const’ qualifier from pointer target type [enabled by default]
> /usr/include/xtables.h:395:23: note: expected ‘struct option *’ but argument
> is of type ‘const struct option *’
> m_xt.c:167:31: warning: passing argument 3 of ‘xtables_merge_options’ from
> incompatible pointer type [enabled by default]
> /usr/include/xtables.h:395:23: note: expected ‘const struct option *’ but
> argument is of type ‘unsigned int *’
> m_xt.c:167:31: error: too few arguments to function ‘xtables_merge_options’
> /usr/include/xtables.h:395:23: note: declared here
> m_xt.c:127:6: warning: variable ‘res’ set but not used [-Wunused-but-set-
> variable]
> m_xt.c: In function ‘print_ipt’:
> m_xt.c:312:30: warning: passing argument 2 of ‘xtables_merge_options’ discards
> ‘const’ qualifier from pointer target type [enabled by default]
> /usr/include/xtables.h:395:23: note: expected ‘struct option *’ but argument
> is of type ‘const struct option *’
> m_xt.c:312:30: warning: passing argument 3 of ‘xtables_merge_options’ from
> incompatible pointer type [enabled by default]
> /usr/include/xtables.h:395:23: note: expected ‘const struct option *’ but
> argument is of type ‘unsigned int *’
> m_xt.c:312:30: error: too few arguments to function ‘xtables_merge_options’
> /usr/include/xtables.h:395:23: note: declared here
> make[1]: *** [m_xt.so] Błąd 1
> rm emp_ematch.lex.c emp_ematch.yacc.c
Backward compatibility was broken in the following iptables commit:
From 600f38db82548a683775fd89b6e136673e924097 Mon Sep 17 00:00:00 2001
From: Jan Engelhardt <jengelh@medozas.de>
Date: Fri, 29 Oct 2010 18:57:42 +0200
Subject: [PATCH] libxtables: change option precedence order to be intuitive
^ permalink raw reply
* Re: [PATCH 1/1] IPVS : bug in ip_vs_ftp, same list heaad used in all netns.
From: Pablo Neira Ayuso @ 2011-05-27 9:33 UTC (permalink / raw)
To: Simon Horman
Cc: Hans Schillstrom, ja@ssi.bg, wensong@linux-vs.org,
lvs-devel@vger.kernel.org, netdev@vger.kernel.org,
netfilter-devel@vger.kernel.org, hans@schillstrom.com,
David Miller
In-Reply-To: <20110527060409.GB24641@verge.net.au>
On 27/05/11 08:04, Simon Horman wrote:
> On Fri, May 27, 2011 at 07:24:22AM +0200, Hans Schillstrom wrote:
>> On Friday 27 May 2011 01:37:45 Simon Horman wrote:
>>> On Thu, May 26, 2011 at 07:17:50PM +0200, Pablo Neira Ayuso wrote:
>>>> On 24/05/11 14:11, Hans Schillstrom wrote:
>>>>> When ip_vs was adapted to netns the ftp application was not adapted
>>>>> in a correct way.
>>>>> However this is a fix to avoid kernel errors. In the long term another solution
>>>>> might be chosen. I.e the ports that the ftp appl, uses should be per netns.
>>>>
>>>> applied, thanks.
>>>
>>> Thanks Pablo.
>>>
>>> Hans, is this appropriate for -stable (i.e. 2.6.39.x) ?
>>
>> Yes it is.
>
> Thanks.
>
> Dave, can you handle that once this change makes
> it into your tree?
http://permalink.gmane.org/gmane.linux.kernel.wireless.general/70374
^ permalink raw reply
* Re: [PATCH] ipv4: fix fib metrics
From: David Woodhouse @ 2011-05-27 9:27 UTC (permalink / raw)
To: Alessandro Suardi, Hiroyuki Kawakatsu
Cc: Kyle Moffett, Eric Dumazet, David Miller, linux-kernel, netdev
In-Reply-To: <AANLkTinQq=BAVS1FrH80+VukTfAK1OKOkjEH4=gwZkv6@mail.gmail.com>
On Fri, 2011-03-25 at 02:25 +0100, Alessandro Suardi wrote:
>
> ...which didn't take that long - one last bugging question and I'm happily
> off to sleep; does ipid always come in the form of 0x followed by four
> bytes representing hex values ? In a perhaps inelegant but working way
> (I'm now writing through the VPN tunnel),
>
> sed 's/cache//;s/metric \?[0-9]\+ [0-9]\+//g;s/hoplimit
> [0-9]\+//g;s/ipid 0x....//g'
>
> appears to be Work For Me (TM).
Please could I have a tested patch for vpnc-script?
It now lives in its own repository at
git://, http://git.infradead.org/users/dwmw2/vpnc-scripts.git because
it's used by openconnect too, and has had various bug fixes for
cross-platform support and IPv6 since it was forked from vpnc.
--
dwmw2
^ permalink raw reply
* [RFC PATCH] net: ethtool: use C99 array initialization for feature-names table
From: Michał Mirosław @ 2011-05-27 9:25 UTC (permalink / raw)
To: netdev
In-Reply-To: <BANLkTik5aF-CafhXSuTOZdCZuJiyz_Ot_A@mail.gmail.com>
This depends on Mahesh Bandewar's cleanup of feature bits.
Note: This is why I'd like that patch to include TSO bits as well,
so I can get rid of this ugly NETIF_F_GSO_SHIFT+n.
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
net/core/ethtool.c | 65 +++++++++++++++++++++++++--------------------------
1 files changed, 32 insertions(+), 33 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 59255c1..e5de5e1 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -350,42 +350,41 @@ static int ethtool_set_features(struct net_device *dev, void __user *useraddr)
return ret;
}
-static const char netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GSTRING_LEN] = {
- /* NETIF_F_SG */ "tx-scatter-gather",
- /* NETIF_F_IP_CSUM */ "tx-checksum-ipv4",
- /* NETIF_F_NO_CSUM */ "tx-checksum-unneeded",
- /* NETIF_F_HW_CSUM */ "tx-checksum-ip-generic",
- /* NETIF_F_IPV6_CSUM */ "tx-checksum-ipv6",
- /* NETIF_F_HIGHDMA */ "highdma",
- /* NETIF_F_FRAGLIST */ "tx-scatter-gather-fraglist",
- /* NETIF_F_HW_VLAN_TX */ "tx-vlan-hw-insert",
+static const char netdev_features_strings[ND_FEATURE_NUM_BITS][ETH_GSTRING_LEN] = {
+ [NETIF_F_SG_BIT] = "tx-scatter-gather",
+ [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
+ [NETIF_F_NO_CSUM_BIT] = "tx-checksum-unneeded",
+ [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
+ [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
+ [NETIF_F_HIGHDMA_BIT] = "highdma",
+ [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
+ [NETIF_F_HW_VLAN_TX_BIT] = "tx-vlan-hw-insert",
- /* NETIF_F_HW_VLAN_RX */ "rx-vlan-hw-parse",
- /* NETIF_F_HW_VLAN_FILTER */ "rx-vlan-filter",
- /* NETIF_F_VLAN_CHALLENGED */ "vlan-challenged",
- /* NETIF_F_GSO */ "tx-generic-segmentation",
- /* NETIF_F_LLTX */ "tx-lockless",
- /* NETIF_F_NETNS_LOCAL */ "netns-local",
- /* NETIF_F_GRO */ "rx-gro",
- /* NETIF_F_LRO */ "rx-lro",
+ [NETIF_F_HW_VLAN_RX_BIT] = "rx-vlan-hw-parse",
+ [NETIF_F_HW_VLAN_FILTER_BIT] = "rx-vlan-filter",
+ [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
+ [NETIF_F_GSO_BIT] = "tx-generic-segmentation",
+ [NETIF_F_LLTX_BIT] = "tx-lockless",
+ [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
+ [NETIF_F_GRO_BIT] = "rx-gro",
+ [NETIF_F_LRO_BIT] = "rx-lro",
- /* NETIF_F_TSO */ "tx-tcp-segmentation",
- /* NETIF_F_UFO */ "tx-udp-fragmentation",
- /* NETIF_F_GSO_ROBUST */ "tx-gso-robust",
- /* NETIF_F_TSO_ECN */ "tx-tcp-ecn-segmentation",
- /* NETIF_F_TSO6 */ "tx-tcp6-segmentation",
- /* NETIF_F_FSO */ "tx-fcoe-segmentation",
- "",
- "",
+ /* must match SKB_GSO_* order */
+ [NETIF_F_GSO_SHIFT+0] = "tx-tcp-segmentation",
+ [NETIF_F_GSO_SHIFT+1] = "tx-udp-fragmentation",
+ [NETIF_F_GSO_SHIFT+2] = "tx-gso-robust",
+ [NETIF_F_GSO_SHIFT+3] = "tx-tcp-ecn-segmentation",
+ [NETIF_F_GSO_SHIFT+4] = "tx-tcp6-segmentation",
+ [NETIF_F_GSO_SHIFT+5] = "tx-fcoe-segmentation",
- /* NETIF_F_FCOE_CRC */ "tx-checksum-fcoe-crc",
- /* NETIF_F_SCTP_CSUM */ "tx-checksum-sctp",
- /* NETIF_F_FCOE_MTU */ "fcoe-mtu",
- /* NETIF_F_NTUPLE */ "rx-ntuple-filter",
- /* NETIF_F_RXHASH */ "rx-hashing",
- /* NETIF_F_RXCSUM */ "rx-checksum",
- /* NETIF_F_NOCACHE_COPY */ "tx-nocache-copy",
- /* NETIF_F_LOOPBACK */ "loopback",
+ [NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc",
+ [NETIF_F_SCTP_CSUM_BIT] = "tx-checksum-sctp",
+ [NETIF_F_FCOE_MTU_BIT] = "fcoe-mtu",
+ [NETIF_F_NTUPLE_BIT] = "rx-ntuple-filter",
+ [NETIF_F_RXHASH_BIT] = "rx-hashing",
+ [NETIF_F_RXCSUM_BIT] = "rx-checksum",
+ [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy",
+ [NETIF_F_LOOPBACK_BIT] = "loopback",
};
static int __ethtool_get_sset_count(struct net_device *dev, int sset)
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH 2/6] unicore32: add pkunity-v3 mac/net driver (umal)
From: Arnd Bergmann @ 2011-05-27 9:19 UTC (permalink / raw)
To: GuanXuetao; +Cc: linux-kernel, linux-arch, greg, netdev
In-Reply-To: <556924396ee34f4af94a2f03767973f1cec35e22.1306408804.git.gxt@mprc.pku.edu.cn>
On Thursday 26 May 2011, GuanXuetao wrote:
> From: Guan Xuetao <gxt@mprc.pku.edu.cn>
>
> Signed-off-by: Guan Xuetao <gxt@mprc.pku.edu.cn>
> ---
> MAINTAINERS | 1 +
> arch/unicore32/configs/debug_defconfig | 2 +-
> drivers/net/Kconfig | 5 +
> drivers/net/Makefile | 1 +
> drivers/net/mac-puv3.c | 1942 ++++++++++++++++++++++++++++++++
> 5 files changed, 1950 insertions(+), 1 deletions(-)
> create mode 100644 drivers/net/mac-puv3.c
I also have a few comments after looking through the driver.
> +
> +/**********************************************************************
> + * Globals
> + ********************************************************************* */
Regular commenting style would be
/*
* Globals
*/
> +/**********************************************************************
> + * Prototypes
> + ********************************************************************* */
> +static int umal_mii_reset(struct mii_bus *bus);
> +static int umal_mii_read(struct mii_bus *bus, int phyaddr, int regidx);
> +static int umal_mii_write(struct mii_bus *bus, int phyaddr, int regidx,
> + u16 val);
> +static int umal_mii_probe(struct net_device *dev);
> +
> +static void umaldma_initctx(struct umaldma *d, struct umal_softc *s,
> + int rxtx, int maxdescr);
> +static void umaldma_uninitctx(struct umaldma *d);
> +static void umaldma_channel_start(struct umaldma *d, int rxtx);
> +static void umaldma_channel_stop(struct umaldma *d);
> +static int umaldma_add_rcvbuffer(struct umal_softc *sc, struct umaldma *d,
> + struct sk_buff *m);
> +static int umaldma_add_txbuffer(struct umaldma *d, struct sk_buff *m);
> +static void umaldma_emptyring(struct umaldma *d);
> +static void umaldma_fillring(struct umal_softc *sc, struct umaldma *d);
> +static int umaldma_rx_process(struct umal_softc *sc, struct umaldma *d,
> + int work_to_do, int poll);
> +static void umaldma_tx_process(struct umal_softc *sc, struct umaldma *d,
> + int poll);
> +
> +static int umal_initctx(struct umal_softc *s);
> +static void umal_uninitctx(struct umal_softc *s);
> +static void umal_channel_start(struct umal_softc *s);
> +static void umal_channel_stop(struct umal_softc *s);
> +static enum umal_state umal_set_channel_state(struct umal_softc *,
> + enum umal_state);
> +
> +static int umal_init(struct platform_device *pldev, long long base);
> +static int umal_open(struct net_device *dev);
> +static int umal_close(struct net_device *dev);
> +static int umal_mii_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
> +static irqreturn_t umal_intr(int irq, void *dev_instance);
> +static void umal_clr_intr(struct net_device *dev);
> +static int umal_start_tx(struct sk_buff *skb, struct net_device *dev);
> +static void umal_tx_timeout(struct net_device *dev);
> +static void umal_set_rx_mode(struct net_device *dev);
> +static void umal_promiscuous_mode(struct umal_softc *sc, int onoff);
> +static void umal_setmulti(struct umal_softc *sc);
> +static int umal_set_speed(struct umal_softc *s, enum umal_speed speed);
> +static int umal_set_duplex(struct umal_softc *s, enum umal_duplex duplex,
> + enum umal_fc fc);
> +static int umal_change_mtu(struct net_device *_dev, int new_mtu);
> +static void umal_miipoll(struct net_device *dev);
Best avoid all these prototypes. Instead, reorder the functions in the
driver so you don't need them. That is the order in which reviewers expect
them.
> +/**********************************************************************
> + * UMAL_MII_RESET(bus)
> + *
> + * Reset MII bus.
> + *
> + * Input parameters:
> + * bus - MDIO bus handle
> + *
> + * Return value:
> + * 0 if ok
> + ********************************************************************* */
For extended function documentation, use the kerneldoc style, e.g.
/**
* umal_mii_reset - reset MII bus
*
* @bus: MDIO bus handle
*
* Returns 0
*/
See also Documentation/kernel-doc-nano-HOWTO.txt
> +/**********************************************************************
> + * UMALDMA_RX_PROCESS(sc,d,work_to_do,poll)
> + *
> + * Process "completed" receive buffers on the specified DMA channel.
> + *
> + * Input parameters:
> + * sc - softc structure
> + * d - DMA channel context
> + * work_to_do - no. of packets to process before enabling interrupt
> + * again (for NAPI)
> + * poll - 1: using polling (for NAPI)
> + *
> + * Return value:
> + * nothing
> + ********************************************************************* */
> +static int umaldma_rx_process(struct umal_softc *sc, struct umaldma *d,
> + int work_to_do, int poll)
It seems that you tried to convert the driver to NAPI but did not succeed,
as this function is only called from the interrupt handler directly.
There is usually a significant performance win from using NAPI, so you
should better try again. If you had problems doing that, please ask
on netdev.
> +
> +#ifdef CONFIG_CMDLINE_FORCE
> + eaddr[0] = 0x00;
> + eaddr[1] = 0x25;
> + eaddr[2] = 0x9b;
> + eaddr[3] = 0xff;
> + eaddr[4] = 0x00;
> + eaddr[5] = 0x00;
> +#endif
> +
> + for (i = 0; i < 6; i++)
> + dev->dev_addr[i] = eaddr[i];
You can use random_ether_addr() to generate a working unique MAC address
if the hardware does not provide one.
Arnd
^ permalink raw reply
* Re: [PATCH 3/3] net: Kill ratelimit.h dependency in linux/net.h
From: Ingo Molnar @ 2011-05-27 8:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110526.164330.2076981043593394399.davem@davemloft.net>
* David Miller <davem@davemloft.net> wrote:
> Ingo Molnar noticed that we have this unnecessary ratelimit.h
> dependency in linux/net.h, which hid compilation problems from
> people doing builds only with CONFIG_NET enabled.
>
> Move this stuff out to a seperate net/net_ratelimit.h file and
> include that in the only two places where this thing is needed.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> include/linux/net.h | 6 ------
> net/core/sysctl_net_core.c | 1 +
> net/core/utils.c | 1 +
> 3 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/net.h b/include/linux/net.h
> index 1da55e9..b299230 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -289,11 +289,5 @@ extern int kernel_sock_shutdown(struct socket *sock,
> MODULE_ALIAS("net-pf-" __stringify(pf) "-proto-" __stringify(proto) \
> "-type-" __stringify(type))
>
> -#ifdef CONFIG_SYSCTL
> -#include <linux/sysctl.h>
> -#include <linux/ratelimit.h>
> -extern struct ratelimit_state net_ratelimit_state;
> -#endif
> -
Assuming that this moved into net_ratelimit.h with a guard define
this looks good to me:
Acked-by: Ingo Molnar <mingo@elte.hu>
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v4 1/1] can: add pruss CAN driver.
From: Wolfgang Grandegger @ 2011-05-27 8:31 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Arnd Bergmann, Subhasish Ghosh, nsekhar-l0cyMroinI0, open list,
CAN NETWORK DRIVERS, Marc Kleine-Budde,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Netdev-u79uwXL29TY76Z2rM5mHXA, m-watkins-l0cyMroinI0, Alan Cox
In-Reply-To: <4DD9FCFC.10803-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Hi Oliver,
sorry for the late answer.
On 05/23/2011 08:21 AM, Oliver Hartkopp wrote:
> On 22.05.2011 12:30, Arnd Bergmann wrote:
>> On Thursday 12 May 2011 16:41:58 Oliver Hartkopp wrote:
>>> E.g. assume you need the CAN-IDs 0x100, 0x200 and 0x300 in your application
>>> and for that reason you configure these IDs in the pruss CAN driver.
>>>
>>> What if someone generates a 100% CAN busload exactly on CAN-ID 0x100 then?
>>>
>>> Worst case (1MBit/s, DLC=0) you would need to handle about 21.000 irqs/s for
>>> the correctly received CAN frames with the filtered CAN-ID 0x100 ...
>>
>> Then I guess the main thing that a "smart" CAN implementation like pruss
>> should do is interrupt mitigation. When you have a constant flow of
>> packets coming in, the hardware should be able to DMA a lot of
>> them into kernel memory before the driver is required to pick them up,
>> and only get into interrupt driven mode when the kernel has managed
>> to process all outstanding packets.
>>
>>> This all depends heavily on Linux networking (skb handling, caching, etc) and
>>> is pretty fast and optimized!! That was also the reason why it ran on the old
>>> PowerPC that smoothly. The mostly seen effect if anything drops is when the
>>> application (holding the socket) was not fast enough to handle the incoming
>>> data. NB: For that reason we implemented a CAN content filter (CAN_BCM) that
>>> is able to do content filtering and timeout monitoring in Kernelspace - all
>>> performed in the SoftIRQ.
>>
>> Right, dropping packets that no process is waiting for should be done as
>> early as possible. In pruss-can, the idea was to do it in hardware, which
>> doesn't really work all that well for the reasons discussed before.
>> Dropping the frames in the NAPI poll function (softirq time) seems like a
>> logical choice.
>
> In 'real world' CAN setups you'll never see 21.000 CAN frames per second (and
> therefore 21.000 irqs/s) - you are usually designing CAN network traffic with
> less than 60% busload. So interrupt rates somewhere below 1000 irqs/s can be
> assumed.
>
>>From what i've seen so far a 3-4 messages rx FIFO and NAPI support just make it.
I think you speak about the SJA100 which is able to buffer 64 byte
corresponding to up to 4 messages. There are CAN controllers able to
queue more or just one message and then NAPI adds overhead.
> @Marc/Wolfgang: Would this be also your recommendation for a CAN controller
> design that supports SocketCAN in the best way?
Anyway, NAPI *always* useful as it helps with the infamous interrupt
flooding.
Wolfgang.
^ permalink raw reply
* [PATCH] caif: Fix compile warning in caif_serial.c
From: Sjur Brændeland @ 2011-05-27 8:09 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Randy Dunlap, linux-next, balbi, netdev, Sjur Brændeland
In-Reply-To: <20110526143006.2d1ef9f7.randy.dunlap@oracle.com>
Fix the compile warning introduced by the patch:
"tty: make receive_buf() return the amout of bytes received"
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
Note: Fixes compile issue in linux-next (no issue in net-2.6).
drivers/net/caif/caif_serial.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 73c7e03..751ebbd 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -191,7 +191,7 @@ static unsigned int ldisc_receive(struct tty_struct *tty,
dev_info(&ser->dev->dev,
"Bytes received before initial transmission -"
"bytes discarded.\n");
- return;
+ return count;
}
BUG_ON(ser->dev == NULL);
@@ -199,7 +199,7 @@ static unsigned int ldisc_receive(struct tty_struct *tty,
/* Get a suitable caif packet and copy in data. */
skb = netdev_alloc_skb(ser->dev, count+1);
if (skb == NULL)
- return;
+ return 0;
p = skb_put(skb, count);
memcpy(p, data, count);
--
1.7.0.4
^ permalink raw reply related
* Re: Kernel crash after using new Intel NIC (igb)
From: Yann Dupont @ 2011-05-27 7:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: Arun Sharma, David Miller, Maximilian Engelhardt, linux-kernel,
netdev, StuStaNet Vorstand, Denys Fedoryshchenko
In-Reply-To: <1306466831.2543.58.camel@edumazet-laptop>
Le 27/05/2011 05:27, Eric Dumazet a écrit :
> Le jeudi 26 mai 2011 à 17:09 -0700, Arun Sharma a écrit :
>> On 5/26/11 3:01 PM, Eric Dumazet wrote:
>>
>>
>>>> Yeah - using the refcnt seems better than list_empty(), but I'm not sure
>>>> that your patch addresses the race above.
>>> It does.
>> True. I can't find any holes in this method and it resolves the "failure
>> to unlink from unused" case.
>>
>> Perhaps wrap the while(1) loop into its own primitive in atomic.h or use
>> an existing primitive?
>>
> Sure, here is a formal submission I cooked.
>
> Thanks
>
> [PATCH] inetpeer: fix race in unused_list manipulations
Thanks eric, didn't noticed this thread, nice to see you squashed this bug.
As you said in a previous message, slub_nomerge prevented us from
crashing for 113 days now :)
But of course, THE REAL FIX is much preffered. Will try this patch with
the next -stable update.
Thanks for your efforts,
--
Yann Dupont - Service IRTS, DSI Université de Nantes
Tel : 02.53.48.49.20 - Mail/Jabber : Yann.Dupont@univ-nantes.fr
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.11
From: Arkadiusz Miskiewicz @ 2011-05-27 7:52 UTC (permalink / raw)
To: shemminger; +Cc: Netfilter Development Mailinglist, NetDev
In-Reply-To: <4DDE8814.7090201@trash.net>
On Thursday 26 of May 2011, Patrick McHardy wrote:
> Am 26.05.2011 18:53, schrieb Patrick McHardy:
> > The netfilter coreteam presents:
> > iptables version 1.4.10
>
> That's supposed to read 1.4.11 of course :)
Too bad it breaks iproute2 build, hope to see fixed iproute2 release then
gcc -D_GNU_SOURCE -O2 -Wstrict-prototypes -Wall -I../include -
DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib/\" -DCONFIG_GACT -DCONFIG_GACT_PROB -
DIPT_LIB_DIR=\"/usr/lib64/xtables\" -Wl,-export-dynamic -shared -fpic -o
q_atm.so q_atm.c -latm
gcc -D_GNU_SOURCE -O2 -Wstrict-prototypes -Wall -I../include -
DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib/\" -DCONFIG_GACT -DCONFIG_GACT_PROB -
DIPT_LIB_DIR=\"/usr/lib64/xtables\" -Wl,-export-dynamic -shared -fpic -o
m_xt.so m_xt.c -lxtables
m_xt.c: In function ‘parse_ipt’:
m_xt.c:167:31: warning: passing argument 2 of ‘xtables_merge_options’ discards
‘const’ qualifier from pointer target type [enabled by default]
/usr/include/xtables.h:395:23: note: expected ‘struct option *’ but argument
is of type ‘const struct option *’
m_xt.c:167:31: warning: passing argument 3 of ‘xtables_merge_options’ from
incompatible pointer type [enabled by default]
/usr/include/xtables.h:395:23: note: expected ‘const struct option *’ but
argument is of type ‘unsigned int *’
m_xt.c:167:31: error: too few arguments to function ‘xtables_merge_options’
/usr/include/xtables.h:395:23: note: declared here
m_xt.c:127:6: warning: variable ‘res’ set but not used [-Wunused-but-set-
variable]
m_xt.c: In function ‘print_ipt’:
m_xt.c:312:30: warning: passing argument 2 of ‘xtables_merge_options’ discards
‘const’ qualifier from pointer target type [enabled by default]
/usr/include/xtables.h:395:23: note: expected ‘struct option *’ but argument
is of type ‘const struct option *’
m_xt.c:312:30: warning: passing argument 3 of ‘xtables_merge_options’ from
incompatible pointer type [enabled by default]
/usr/include/xtables.h:395:23: note: expected ‘const struct option *’ but
argument is of type ‘unsigned int *’
m_xt.c:312:30: error: too few arguments to function ‘xtables_merge_options’
/usr/include/xtables.h:395:23: note: declared here
make[1]: *** [m_xt.so] Błąd 1
rm emp_ematch.lex.c emp_ematch.yacc.c
--
Arkadiusz Miśkiewicz PLD/Linux Team
arekm / maven.pl http://ftp.pld-linux.org/
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.11
From: Maciej Żenczykowski @ 2011-05-27 7:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: Patrick McHardy, Netfilter Development Mailinglist, NetDev,
netfilter-announce, netfilter@vger.kernel.org
In-Reply-To: <1306434537.2543.4.camel@edumazet-laptop>
you could try with -M '' (or something like that) if you want to
prevent even xtables from being loaded.
Although that will probably still not prevent iptable_filter from
being loaded if ip_tables is already loaded...
On Thu, May 26, 2011 at 20:28, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 26 mai 2011 à 18:53 +0200, Patrick McHardy a écrit :
>> The netfilter coreteam presents:
>>
>> iptables version 1.4.10
>>
>> the iptables release for the 2.6.39 kernels. Due to some mistakes
>> on my side we didn't have a release for longer than expected, so
>> this contains a rather large number of changes.
>>
>> Changes include:
>>
>
> ...
>> - a new iptables option "-C" to check for existance of a rules
>
> Nice, but this still loads modules...
>
> # lsmod | grep ipta
> # ./iptables -C INPUT -p tcp
> iptables: Bad rule (does a matching rule exist in that chain?).
> # lsmod | grep ipta
> iptable_filter 1730 0
> ip_tables 15958 1 iptable_filter
> x_tables 22998 3 iptable_filter,ip_tables,xt_tcpudp
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [patch 1/1] net: convert %p usage to %pK
From: Ingo Molnar @ 2011-05-27 6:37 UTC (permalink / raw)
To: Eric Dumazet, Dan Rosenberg
Cc: David Miller, kees.cook, joe, akpm, netdev, drosenberg,
a.p.zijlstra, eparis, eugeneteo, jmorris, tgraf
In-Reply-To: <1306465841.2543.52.camel@edumazet-laptop>
* Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 26 mai 2011 à 22:44 -0400, David Miller a écrit :
> > From: Kees Cook <kees.cook@canonical.com>
> > Date: Thu, 26 May 2011 17:14:49 -0700
> > >
> > > We got this dropped from the /proc view; why can't we do the same for
> > > this netlink interface?
> >
> > Because it's not only an opaque "output" blob, it's also an input key
> > for lookups which the user can trigger.
>
> Yes, we wan add a layer to obfuscate the real pointers. We dont
> trust values given by user, only match them.
>
> Either we use a XOR with a boot time random value (but let the NULL
> cookie being the NULL one), or we generate an unique 64bit socket
> id for the cookie (and keep a 64bit cookie in all sockets,
> increasing ram usage)
FYI, Dan Rosenberg is currently working on a kernel image
randomization feature, see this lkml thread:
[RFC][PATCH] Randomize kernel base address on boot
That will come with an easy vsprintf method to 'unrandomize' IPs.
( this will be used to display a real-looking /proc/kallsyms and all
IPs that the kernel passes to user-space (via perf, etc.) will be
unrandomized as well, protecting the randomization seed. )
Once that code goes upstream the networking code could rather simply
use it to 'randomize' these real data pointers as well. (Assuming you
never ever pass in zero, that would expose the secret seed.)
The only worry would be statistical analysis performed by local
attackers: by creating and closing enough sockets on a busy system
you can over time cover almost all ranges of RAM, so if you can
observe a pattern of 'over the address space limit' addresses at the
top or the bottom of the address space you can estimate the random
seed to within 4-5 bits realistically, maybe even more.
The upper and lower limit of the address space (big holes are useful
too) can be calculated rather precisely based on RAM size, the
identity of the system and the kernel version.
So maybe networking real-pointer randomization should be separate
from kernel IP randomization after all.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH 1/1] IPVS : bug in ip_vs_ftp, same list heaad used in all netns.
From: Simon Horman @ 2011-05-27 6:04 UTC (permalink / raw)
To: Hans Schillstrom
Cc: Pablo Neira Ayuso, ja@ssi.bg, wensong@linux-vs.org,
lvs-devel@vger.kernel.org, netdev@vger.kernel.org,
netfilter-devel@vger.kernel.org, hans@schillstrom.com,
David Miller
In-Reply-To: <201105270724.24129.hans.schillstrom@ericsson.com>
On Fri, May 27, 2011 at 07:24:22AM +0200, Hans Schillstrom wrote:
> On Friday 27 May 2011 01:37:45 Simon Horman wrote:
> > On Thu, May 26, 2011 at 07:17:50PM +0200, Pablo Neira Ayuso wrote:
> > > On 24/05/11 14:11, Hans Schillstrom wrote:
> > > > When ip_vs was adapted to netns the ftp application was not adapted
> > > > in a correct way.
> > > > However this is a fix to avoid kernel errors. In the long term another solution
> > > > might be chosen. I.e the ports that the ftp appl, uses should be per netns.
> > >
> > > applied, thanks.
> >
> > Thanks Pablo.
> >
> > Hans, is this appropriate for -stable (i.e. 2.6.39.x) ?
>
> Yes it is.
Thanks.
Dave, can you handle that once this change makes
it into your tree?
^ permalink raw reply
* Re: [Bugme-new] [Bug 35862] New: arp requests from wrong src IP
From: Julian Anastasov @ 2011-05-27 5:27 UTC (permalink / raw)
To: Victor Mataré
Cc: David Miller, akpm, netdev, bugzilla-daemon, bugme-daemon
In-Reply-To: <201105260400.22814.matare@lih.rwth-aachen.de>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1115 bytes --]
Hello,
On Thu, 26 May 2011, Victor Mataré wrote:
> Examining the host which now has 137.226.164.2 (used to have 137.226.164.13):
>
> # ip addr show dev eth0
> 4: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP qlen 1000
> link/ether 00:e0:81:41:1f:e4 brd ff:ff:ff:ff:ff:ff
> inet 137.226.164.2/24 brd 137.226.164.255 scope global eth0
> inet 192.168.23.2/24 brd 137.226.164.255 scope global eth0:0
> [...]
>
> Sorry, got confused with all the swapping. I'm *not* keeping the old address around, it's completely *gone*, from both ifconfig and ip. But still it's being used as arp src address. That's what this bug is about. Sorry for the confusion.
It looks strange. Can you confirm the following things:
- the kernel version
- the order of 'ip' command used to add and change IPs on this box
- output of 'ip route list table local' after IPs are changed and
before starting arping
- output of 'strace arping', I assume it is using getsockname
after UDP connect
- any reason to use broadcast 137.226.164.255 for all addresses?
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH 1/1] IPVS : bug in ip_vs_ftp, same list heaad used in all netns.
From: Hans Schillstrom @ 2011-05-27 5:24 UTC (permalink / raw)
To: Simon Horman
Cc: Pablo Neira Ayuso, ja@ssi.bg, wensong@linux-vs.org,
lvs-devel@vger.kernel.org, netdev@vger.kernel.org,
netfilter-devel@vger.kernel.org, hans@schillstrom.com
In-Reply-To: <20110526233744.GD2815@verge.net.au>
On Friday 27 May 2011 01:37:45 Simon Horman wrote:
> On Thu, May 26, 2011 at 07:17:50PM +0200, Pablo Neira Ayuso wrote:
> > On 24/05/11 14:11, Hans Schillstrom wrote:
> > > When ip_vs was adapted to netns the ftp application was not adapted
> > > in a correct way.
> > > However this is a fix to avoid kernel errors. In the long term another solution
> > > might be chosen. I.e the ports that the ftp appl, uses should be per netns.
> >
> > applied, thanks.
>
> Thanks Pablo.
>
> Hans, is this appropriate for -stable (i.e. 2.6.39.x) ?
Yes it is.
--
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>
^ permalink raw reply
* RE: [PATCH 2/6] unicore32: add pkunity-v3 mac/net driver (umal)
From: Guan Xuetao @ 2011-05-27 4:27 UTC (permalink / raw)
To: 'Ben Hutchings'; +Cc: arnd, linux-kernel, linux-arch, greg, netdev
In-Reply-To: <1306432569.17233.123.camel@localhost>
> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Friday, May 27, 2011 1:56 AM
> To: GuanXuetao
> Cc: arnd@arndb.de; linux-kernel@vger.kernel.org; linux-arch@vger.kernel.org; greg@kroah.com; netdev@vger.kernel.org
> Subject: Re: [PATCH 2/6] unicore32: add pkunity-v3 mac/net driver (umal)
>
> On Thu, 2011-05-26 at 11:36 +0000, GuanXuetao wrote:
> [...]
> > --- /dev/null
> > +++ b/drivers/net/mac-puv3.c
> [...]
> > +/**********************************************************************
> > + * Simple types
> > + ********************************************************************* */
> > +enum umal_speed {
> > + umal_speed_none = 0,
> > + umal_speed_10 = SPEED_10,
> > + umal_speed_100 = SPEED_100,
> > + umal_speed_1000 = SPEED_1000,
> > +};
>
> Just use the numbers directly: 0, 10, 100, 1000.
Applied.
>
> [...]
> > +#define ETHER_ADDR_LEN 6
>
> This is already called ETH_ALEN.
Applied.
>
> [...]
> > +/**********************************************************************
> > + * DMA Descriptor structure
> > + ********************************************************************* */
> > +struct umaldmadscr {
> > + dma_addr_t PacketStartAddr;
> > + int PacketSize;
> > + dma_addr_t NextDescriptor;
> > + struct umaldmadscr *NextDescriptor_Virt;
> > +};
>
> Linux naming convention for structure fields is words_with_underscores
> not TitleCase.
Ok, thanks.
>
> [...]
> > +/**********************************************************************
> > + * UMAL_MII_RESET(bus)
> > + *
> > + * Reset MII bus.
> > + *
> > + * Input parameters:
> > + * bus - MDIO bus handle
> > + *
> > + * Return value:
> > + * 0 if ok
> > + ********************************************************************* */
>
> Function documentation comments must follow the kernel-doc format.
Ok, thanks.
>
> [...]
> > +static int umaldma_add_rcvbuffer(struct umal_softc *sc, struct umaldma *d,
> > + struct sk_buff *sb)
> > +{
> [...]
> > + /*
> > + * Allocate a sk_buff if we don't already have one.
> > + * If we do have an sk_buff, reset it so that it's empty.
> > + *
> > + * Note: sk_buffs don't seem to be guaranteed to have any sort
> > + * of alignment when they are allocated. Therefore, allocate enough
> > + * extra space to make sure that:
> > + *
> > + * 1. the data does not start in the middle of a cache line.
> > + * 2. The data does not end in the middle of a cache line
> > + * 3. The buffer can be aligned such that the IP addresses are
> > + * naturally aligned.
> > + *
> > + * Remember, the SOCs MAC writes whole cache lines at a time,
> > + * without reading the old contents first. So, if the sk_buff's
> > + * data portion starts in the middle of a cache line, the SOC
> > + * DMA will trash the beginning (and ending) portions.
> > + */
> > + if (sb == NULL) {
> > + sb_new = netdev_alloc_skb(dev, ENET_PACKET_SIZE +
> > + SMP_CACHE_BYTES * 2 + NET_IP_ALIGN);
> > + if (sb_new == NULL) {
> > + pr_info("%s: sk_buff allocation failed\n",
> > + d->umaldma_eth->umal_dev->name);
> > + return -ENOBUFS;
>
> Surely -ENOMEM.
Applied.
>
> > + }
> > + skb_reserve(sb_new, 2);
>
> This presumably needs to be:
>
> skb_reserve(sb_new, SMP_CACHE_BYTES + NET_IP_ALIGN);
>
> unless you assume that the skb allocator will always ensure cache line
> alignment (in which case, why use padding of SMP_CACHE_BYTES * 2?).
Applied, thanks.
Each packet address needs to be added by NET_IP_ALIGN.
And I think that SMP_CACHE_BYTES is also necessary to guarantee correctness
when invalidate dma cache lines.
>
> [...]
> > +static void umaldma_tx_process(struct umal_softc *sc, struct umaldma *d,
> > + int poll)
> > +{
> > + struct net_device *dev = sc->umal_dev;
> > + int curidx;
> > + int hwidx;
> > + struct umaldmadscr *dsc;
> > + struct sk_buff *sb;
> > + unsigned long flags;
> > + int packets_handled = 0;
> > + unsigned int int_status;
> > +
> > + spin_lock_irqsave(&(sc->umal_lock), flags);
> > +
> > + if (!netif_device_present(dev))
> > + return;
> [...]
>
> This returns with the lock held!
Applied, thanks.
>
> (This is not by any means a thorough review.)
>
> Ben.
>
> --
> Ben Hutchings, Senior Software Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
Thanks Ben.
I will submit next version after correcting the coding-style.
Guan Xuetao
^ permalink raw reply
* Re: [PATCH 1/2 v2] af-packet: Use existing netdev reference for bound sockets.
From: Ben Greear @ 2011-05-27 4:11 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1306467745.2543.60.camel@edumazet-laptop>
On 05/26/2011 08:42 PM, Eric Dumazet wrote:
> Le jeudi 26 mai 2011 à 16:55 -0700, greearb@candelatech.com a écrit :
>> out_free:
>> kfree_skb(skb);
>> out_unlock:
>> - if (dev)
>> + if (dev&& need_rls_dev)
>> dev_put(dev);
>> out:
>> return err;
>
> Hmmm, I wonder why you want this Ben.
>
> IMHO this is buggy, because we can sleep in this function.
>
> We must take a ref on device (its really cheap these days, now we have a
> percpu device refcnt)
Why must you take the reference? And if we must, why isn't the
current code that assigns the prot_hook.dev without taking a
reference OK?
It seems a waste to do the lookup and free if we don't have to,
and with thousands of devices, the lookup might take a reasonable
amount of effort?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ 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