* Re: [PATCH RESEND net-next v2 1/3] bonding: update the primary slave when changing slave's name
From: Ding Tianhong @ 2014-01-14 12:52 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: Jay Vosburgh, David S. Miller, Netdev
In-Reply-To: <20140114105112.GC20066@redhat.com>
On 2014/1/14 18:51, Veaceslav Falico wrote:
> On Tue, Jan 14, 2014 at 05:08:21PM +0800, Ding Tianhong wrote:
>> If the slave's name changed, and the bond params primary is exist,
>> the bond should deal with the situation in two ways:
>>
>> 1) If the slave was the primary slave yet, clean the primary slave
>> and reselect active slave.
>> 2) If the slave's new name is as same as bond primary, set the slave
>> as primary slave and reselect active slave.
>>
>> Thanks for Veaceslav's suggestion.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
>> 1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index e06c445..64e25d5 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2860,9 +2860,29 @@ static int bond_slave_netdev_event(unsigned long event,
>> */
>> break;
>> case NETDEV_CHANGENAME:
>> - /*
>> - * TODO: handle changing the primary's name
>> + /* Handle changing the slave's name:
>> + * 1) If the slave was primary slave,
>> + * clean the primary slave and reselect
>> + * active slave.
>> + * 2) If the slave's new name is same as
>> + * bond primary, set the slave as primary
>> + * slave and reselect active slave.
>> */
>> + if (slave == bond->primary_slave ||
>> + !strcmp(bond->params.primary, slave_dev->name)) {
>
> And if we're in a mode that doesn't use primary, but have the
> params.primary set? Then we'll issue a bond_select_active_slave() in, say,
> 802.3ad mode.
>
> In the past 24h I've nacked about 5 of your patchsets, with you keeping
> 'quickfixing' them, without getting your time to understand the issues, and
> re-sending them for review.
>
> I'm not willing to waste my time that uselessly, reviewing patchsets that
> you randomly generate in the hope of getting it right. And given your
> 'good' history - with patchsets that cause regressions and bugs, with
> reverts because of that, with those horrible, meaningless RCU transition
> that is just plainly wrong and *really* hard to fix - I'm going to react as
> Greg KH said in one of his presentations - NAK your patches and make them
> by myself. It will take *a lot* lesser time from my side, and will
> eventually make the code better.
>
> Thanks for the report, I'll send a patch that fixes it soon.
>
> Nacked-by: Veaceslav Falico <vfalico@redhat.com>
Maybe I am not in the state all day, always miss here and miss there, sorry for that.
>
>> + if (bond->primary_slave) {
>> + pr_info("%s: Setting primary slave to None.\n",
>> + bond->dev->name);
>> + bond->primary_slave = NULL;
>> + } else {
>> + pr_info("%s: Setting %s as primary slave.\n",
>> + bond->dev->name, slave_dev->name);
>> + bond->primary_slave = slave;
>> + }
>> + write_lock_bh(&bond->curr_slave_lock);
>> + bond_select_active_slave(bond);
>> + write_unlock_bh(&bond->curr_slave_lock);
>> + }
>> break;
>> case NETDEV_FEAT_CHANGE:
>> bond_compute_features(bond);
>> --
>> 1.8.0
>>
>>
>
> .
>
^ permalink raw reply
* Re: [PATCH net-next 07/10] batman-adv: use __dev_get_by_index instead of dev_get_by_index to find interface
From: Antonio Quartulli @ 2014-01-14 12:43 UTC (permalink / raw)
To: Ying Xue, davem
Cc: vfalico, john.r.fastabend, stephen, dmitry.tarnyagin, socketcan,
johannes, netdev,
The list for a Better Approach To Mobile Ad-hoc Networking
In-Reply-To: <1389685269-18600-8-git-send-email-ying.xue@windriver.com>
[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]
On 14/01/14 08:41, Ying Xue wrote:
> The following call chains indicate that batadv_is_on_batman_iface()
> is always under rtnl_lock protection as call_netdevice_notifier()
> is protected by rtnl_lock. So if __dev_get_by_index() rather than
> dev_get_by_index() is used to find interface handler in it, this
> would help us avoid to change interface reference counter.
>
> call_netdevice_notifier()
> batadv_hard_if_event()
> batadv_hardif_add_interface()
> batadv_is_valid_iface()
> batadv_is_on_batman_iface()
>
> Cc: Antonio Quartulli <antonio@meshcoding.com>
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
Acked-by: Antonio Quartulli <antonio@meshcoding.com>
> ---
> net/batman-adv/hard-interface.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
> index bebd46c..115d14e 100644
> --- a/net/batman-adv/hard-interface.c
> +++ b/net/batman-adv/hard-interface.c
> @@ -86,15 +86,13 @@ static bool batadv_is_on_batman_iface(const struct net_device *net_dev)
> return false;
>
> /* recurse over the parent device */
> - parent_dev = dev_get_by_index(&init_net, net_dev->iflink);
> + parent_dev = __dev_get_by_index(&init_net, net_dev->iflink);
> /* if we got a NULL parent_dev there is something broken.. */
> if (WARN(!parent_dev, "Cannot find parent device"))
> return false;
>
> ret = batadv_is_on_batman_iface(parent_dev);
>
> - if (parent_dev)
> - dev_put(parent_dev);
> return ret;
> }
>
>
--
Antonio Quartulli
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [ovs-dev] [GIT net-next] Open vSwitch
From: Zoltan Kiss @ 2014-01-14 12:31 UTC (permalink / raw)
To: Thomas Graf, Jesse Gross; +Cc: David Miller, dev@openvswitch.org, netdev
In-Reply-To: <52D50767.6050906@redhat.com>
On 14/01/14 09:46, Thomas Graf wrote:
> On 01/14/2014 02:30 AM, Jesse Gross wrote:
>>> And it works. I guess the last one causing the problem. Might be an
>>> important factor, I'm using 32 bit Dom0.
>>
>> I think you're probably right. Thomas - can you take a look?
>>
>> We shouldn't be doing any zerocopy in this situation but it looks to
>> me like we don't do any padding at all, even in situations where we
>> are copying the data.
>
> I'm looking into this now. The zerocopy method should only be attempted
> if user space has announced the ability to received unaligned messages.
>
> @Zoltan: I assume you are using an unmodified OVS 1.9.3?
>
Well, there are a few changes, but none of them seems to be significant.
Most of them are just making OVS work with our build system. Here is the
patchqueue:
https://github.com/xenserver/openvswitch.pg/tree/master/master
See the series file for the order of the patches.
Zoli
^ permalink raw reply
* Re: [PATCH net-next] net: make dev_set_mtu() honor notification return code
From: Veaceslav Falico @ 2014-01-14 12:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jiri, edumazet, alexander.h.duyck, nicolas.dichtel
In-Reply-To: <20140113.151855.1046745397621207165.davem@davemloft.net>
On Mon, Jan 13, 2014 at 03:18:55PM -0800, David Miller wrote:
>From: Veaceslav Falico <vfalico@redhat.com>
>Date: Fri, 10 Jan 2014 13:48:17 +0100
>
>> Currently, after changing the MTU for a device, dev_set_mtu() calls
>> NETDEV_CHANGEMTU notification, however doesn't verify it's return code -
>> which can be NOTIFY_BAD - i.e. some of the net notifier blocks refused this
>> change, and continues nevertheless.
>>
>> To fix this, verify the return code, and if it's an error - then revert the
>> MTU to the original one, and pass the error code.
>>
>> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>
>This is really a precariously designed code path.
>
>If one of the notifiers says NOTIFY_BAD, well we've already changed
>dev->mtu, therefore what if a packet got sent during this time?
>
>Whoever the NOTIFY_BAD signaller is, obviously cannot handle an MTU
>setting which we've already set in the netdev. So allowing it's a
>terribly idea to allow visibility of the new MTU value until we can be
>sure everyone can handle it.
>
>The problem is that we really need a transaction of some sort to fix
>this properly. First, we'd need to ask all the notifiers if they
>can handle the new MTU, then we somehow atomically set netdev->mtu
>and have the notifiers commit their own state changes.
Yeah, but I can't think of a method to atomically set it for both netdev
and its notifiers... As in, for example, bridge (but not only) takes the
lowest MTU of its ports.
>
>Then we'll have the stick issue of what to do if a notifier is
>unregistered between the check and the commit. :-)
Maybe you've meant 'registered between ...' ? :-) Anyway, I guess
dev_set_mtu() should always be called under RTNL, and this way we won't
have these problems. Though I might be wrong, everyone seems playing with
MTU the way they want.
>
>Your patch is an improvement so I will apply it, this stuff really
>is full of holes still.
One (least intrusive) approach would be to add NETDEV_PRECHANGEMTU, which
would be used to verify if the notifiers all agree with changing, and leave
the NETDEV_CHANGEMTU fail only when something really bad happened. That's
your idea, basically.
As, currently, only team can signal NOTIFY_BAD on mtu change, it's really
easy to implement. What do you think?
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 736050d..4e50e04 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -2850,7 +2850,7 @@ static int team_device_event(struct notifier_block *unused,
case NETDEV_FEAT_CHANGE:
team_compute_features(port->team);
break;
- case NETDEV_CHANGEMTU:
+ case NETDEV_PRECHANGEMTU:
/* Forbid to change mtu of underlaying device */
return NOTIFY_BAD;
case NETDEV_PRE_TYPE_CHANGE:
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a2a70cc..7e023c4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1731,6 +1731,7 @@ struct pcpu_sw_netstats {
#define NETDEV_JOIN 0x0014
#define NETDEV_CHANGEUPPER 0x0015
#define NETDEV_RESEND_IGMP 0x0016
+#define NETDEV_PRECHANGEMTU 0x0017
int register_netdevice_notifier(struct notifier_block *nb);
int unregister_netdevice_notifier(struct notifier_block *nb);
diff --git a/net/core/dev.c b/net/core/dev.c
index 87312dc..096d4dd 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5332,6 +5332,10 @@ int dev_set_mtu(struct net_device *dev, int new_mtu)
if (!netif_device_present(dev))
return -ENODEV;
+ err = call_netdevice_notifiers(NETDEV_PRECHANGEMTU, dev);
+ if (!err)
+ return notifier_to_errno(err);
+
orig_mtu = dev->mtu;
err = __dev_set_mtu(dev, new_mtu);
>
>Thanks.
>
^ permalink raw reply related
* Re: [PATCH net-next 10/10] net: nl80211: __dev_get_by_index instead of dev_get_by_index to find interface
From: Johannes Berg @ 2014-01-14 12:11 UTC (permalink / raw)
To: Ying Xue
Cc: davem, vfalico, john.r.fastabend, stephen, antonio,
dmitry.tarnyagin, socketcan, netdev, linux-kernel
In-Reply-To: <1389685269-18600-11-git-send-email-ying.xue@windriver.com>
On Tue, 2014-01-14 at 15:41 +0800, Ying Xue wrote:
> @@ -2218,10 +2194,6 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
> rdev->wiphy.coverage_class = old_coverage_class;
> }
> }
> -
> - bad_res:
> - if (netdev)
> - dev_put(netdev);
> return result;
I believe that could be "return 0;" now, which would make the code
easier to read.
johannes
^ permalink raw reply
* Re: [PATCH net-next 02/10] bonding: use __dev_get_by_name instead of dev_get_by_name to find interface
From: Veaceslav Falico @ 2014-01-14 11:55 UTC (permalink / raw)
To: Ying Xue
Cc: davem, john.r.fastabend, stephen, antonio, dmitry.tarnyagin,
socketcan, johannes, netdev, linux-kernel
In-Reply-To: <1389685269-18600-3-git-send-email-ying.xue@windriver.com>
On Tue, Jan 14, 2014 at 03:41:01PM +0800, Ying Xue wrote:
>The following call chain indicates that bond_do_ioctl() is protected
>under rtnl_lock. If we use __dev_get_by_name() instead of
>dev_get_by_name() to find interface handler in it, this would
>help us avoid to change reference counter of interface once.
>
>dev_ioctl()
> rtnl_lock()
> dev_ifsioc()
> bond_do_ioctl()
> rtnl_unlock()
>
>Additionally we also change the coding style in bond_do_ioctl(),
>letting it more readable for us.
>
>Cc: Jay Vosburgh <fubar@us.ibm.com>
>Cc: Veaceslav Falico <vfalico@redhat.com>
Acked-by: Veaceslav Falico <vfalico@redhat.com>
>Signed-off-by: Ying Xue <ying.xue@windriver.com>
>---
> drivers/net/bonding/bond_main.c | 49 ++++++++++++++++++---------------------
> 1 file changed, 23 insertions(+), 26 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index e06c445..a69afbf 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -3213,37 +3213,34 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
> if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
> return -EPERM;
>
>- slave_dev = dev_get_by_name(net, ifr->ifr_slave);
>+ slave_dev = __dev_get_by_name(net, ifr->ifr_slave);
>
> pr_debug("slave_dev=%p:\n", slave_dev);
>
> if (!slave_dev)
>- res = -ENODEV;
>- else {
>- pr_debug("slave_dev->name=%s:\n", slave_dev->name);
>- switch (cmd) {
>- case BOND_ENSLAVE_OLD:
>- case SIOCBONDENSLAVE:
>- res = bond_enslave(bond_dev, slave_dev);
>- break;
>- case BOND_RELEASE_OLD:
>- case SIOCBONDRELEASE:
>- res = bond_release(bond_dev, slave_dev);
>- break;
>- case BOND_SETHWADDR_OLD:
>- case SIOCBONDSETHWADDR:
>- bond_set_dev_addr(bond_dev, slave_dev);
>- res = 0;
>- break;
>- case BOND_CHANGE_ACTIVE_OLD:
>- case SIOCBONDCHANGEACTIVE:
>- res = bond_option_active_slave_set(bond, slave_dev);
>- break;
>- default:
>- res = -EOPNOTSUPP;
>- }
>+ return -ENODEV;
>
>- dev_put(slave_dev);
>+ pr_debug("slave_dev->name=%s:\n", slave_dev->name);
>+ switch (cmd) {
>+ case BOND_ENSLAVE_OLD:
>+ case SIOCBONDENSLAVE:
>+ res = bond_enslave(bond_dev, slave_dev);
>+ break;
>+ case BOND_RELEASE_OLD:
>+ case SIOCBONDRELEASE:
>+ res = bond_release(bond_dev, slave_dev);
>+ break;
>+ case BOND_SETHWADDR_OLD:
>+ case SIOCBONDSETHWADDR:
>+ bond_set_dev_addr(bond_dev, slave_dev);
>+ res = 0;
>+ break;
>+ case BOND_CHANGE_ACTIVE_OLD:
>+ case SIOCBONDCHANGEACTIVE:
>+ res = bond_option_active_slave_set(bond, slave_dev);
>+ break;
>+ default:
>+ res = -EOPNOTSUPP;
> }
>
> return res;
>--
>1.7.9.5
>
^ permalink raw reply
* [PATCH v2 net-next] bonding: handle slave's name change with primary_slave logic
From: Veaceslav Falico @ 2014-01-14 11:49 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Ding Tianhong, Jay Vosburgh, Andy Gospodarek
Currently, if a slave's name change, we just pass it by. However, if the
slave is a current primary_slave, then we end up with using a slave, whose
name != params.primary, for primary_slave. And vice-versa, if we don't have
a primary_slave but have params.primary set - we will not detected a new
primary_slave.
Fix this by catching the NETDEV_CHANGENAME event and setting primary_slave
accordingly. Also, if the primary_slave was changed, issue a reselection of
the active slave, cause the priorities have changed.
Reported-by: Ding Tianhong <dingtianhong@huawei.com>
CC: Ding Tianhong <dingtianhong@huawei.com>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e06c445..8077199 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2860,9 +2860,26 @@ static int bond_slave_netdev_event(unsigned long event,
*/
break;
case NETDEV_CHANGENAME:
- /*
- * TODO: handle changing the primary's name
- */
+ /* we don't care if we don't have primary set */
+ if (!USES_PRIMARY(bond->params.mode) ||
+ !bond->params.primary[0])
+ break;
+
+ if (slave == bond->primary_slave) {
+ /* slave's name changed - he's no longer primary */
+ bond->primary_slave = NULL;
+ } else if (!strcmp(slave_dev->name, bond->params.primary)) {
+ /* we have a new primary slave */
+ bond->primary_slave = slave;
+ } else /* we didn't change primary - exit */
+ break;
+
+ pr_info("%s: Primary slave changed to %s, re-electing.\n",
+ bond->dev->name, bond->primary_slave ? slave_dev->name :
+ "none");
+ write_lock_bh(&bond->curr_slave_lock);
+ bond_select_active_slave(bond);
+ write_unlock_bh(&bond->curr_slave_lock);
break;
case NETDEV_FEAT_CHANGE:
bond_compute_features(bond);
--
1.8.4
^ permalink raw reply related
* Re: [PATCH net-next] bonding: handle slave's name change with primary_slave logic
From: Veaceslav Falico @ 2014-01-14 11:50 UTC (permalink / raw)
To: netdev; +Cc: Ding Tianhong, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389696645-9101-1-git-send-email-vfalico@redhat.com>
On Tue, Jan 14, 2014 at 11:50:45AM +0100, Veaceslav Falico wrote:
>Currently, if a slave's name change, we just pass it by. However, if the
>slave is a current primary_slave, then we end up with using a slave, whose
>name != params.primary, for primary_slave. And vice-versa, if we don't have
>a primary_slave but have params.primary set - we will not detected a new
>primary_slave.
>
>Fix this by catching the NETDEV_CHANGENAME event and setting primary_slave
>accordingly. Also, if the primary_slave was changed, issue a reselection of
>the active slave, cause the priorities have changed.
>
>Reported-by: Ding Tianhong <dingtianhong@huawei.com>
>CC: Ding Tianhong <dingtianhong@huawei.com>
>CC: Jay Vosburgh <fubar@us.ibm.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
Self-NAK, too busy reviewing other patches.
Sent v2.
>---
> drivers/net/bonding/bond_main.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index e06c445..b16d7ec 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2860,9 +2860,26 @@ static int bond_slave_netdev_event(unsigned long event,
> */
> break;
> case NETDEV_CHANGENAME:
>- /*
>- * TODO: handle changing the primary's name
>- */
>+ /* we don't care if we don't have primary set */
>+ if (!USES_PRIMARY(bond->params.mode) ||
>+ !bond->params.primary[0])
>+ break;
>+
>+ if (slave == bond->primary_slave) {
>+ /* slave's name changed - he's no longer primary */
>+ bond->primary_slave = NULL;
>+ } else if (!strcmp(slave_dev->name, bond->params.primary)( {
>+ /* we have a new primary slave */
>+ bond->primary_slave = slave;
>+ } else /* we didn't change primary - exit */
>+ break;
>+
>+ pr_info("%s: Primary slave changed to %s, re-electing.\n",
>+ bond->dev->name, bond->primary_slave ? slave_dev->name :
>+ "none");
>+ write_lock_bh(&bond->curr_slave_lock);
>+ bond_select_active_slave(bond);
>+ write_unlock_bh(&bond->curr_slave_lock);
> break;
> case NETDEV_FEAT_CHANGE:
> bond_compute_features(bond);
>--
>1.8.4
>
^ permalink raw reply
* Re: [PATCH] sh_eth: fix garbled TX error message
From: Sergei Shtylyov @ 2014-01-14 11:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-sh
In-Reply-To: <20140113.232935.776174177146650322.davem@davemloft.net>
Hello.
On 14-01-2014 11:29, David Miller wrote:
>> sh_eth_error() in case of a TX error tries to print a message using 2 dev_err()
>> calls with the first string not finished by '\n', so that the resulting message
>> would inevitably come out garbled, with something like "3net eth0: " inserted
>> in the middle. Avoid that by merging 2 calls into one.
>> While at it, insert an empty line after the nearby declaration.
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Applied, thanks.
> I don't think this is really -stable material, sorry.
Right, this is not even a fix anymore, at least not from 2.6.31 times (if
Joe's estimate was correct), and I was going to resubmit it as a cleanup.
Should have warned you not to apply yet but didn't, sorry.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] netfilter: Add dependency on IPV6 for NF_TABLES_INET
From: Patrick McHardy @ 2014-01-14 11:35 UTC (permalink / raw)
To: Paul Gortmaker, David S. Miller, Pablo Neira Ayuso,
Jozsef Kadlecsik
Cc: netfilter, netdev
In-Reply-To: <1389636070-26312-1-git-send-email-paul.gortmaker@windriver.com>
Paul Gortmaker <paul.gortmaker@windriver.com> schrieb:
>Commit 1d49144c0aaa61be4e3ccbef9cc5c40b0ec5f2fe ("netfilter: nf_tables:
>add "inet" table for IPv4/IPv6") allows creation of non-IPV6 enabled
>.config files that will fail to configure/link as follows:
>
>warning: (NF_TABLES_INET) selects NF_TABLES_IPV6 which has unmet direct
>dependencies (NET && INET && IPV6 && NETFILTER && NF_TABLES)
>warning: (NF_TABLES_INET) selects NF_TABLES_IPV6 which has unmet direct
>dependencies (NET && INET && IPV6 && NETFILTER && NF_TABLES)
>warning: (NF_TABLES_INET) selects NF_TABLES_IPV6 which has unmet direct
>dependencies (NET && INET && IPV6 && NETFILTER && NF_TABLES)
>net/built-in.o: In function `nft_reject_eval':
>nft_reject.c:(.text+0x651e8): undefined reference to `nf_ip6_checksum'
>nft_reject.c:(.text+0x65270): undefined reference to `ip6_route_output'
>nft_reject.c:(.text+0x656c4): undefined reference to `ip6_dst_hoplimit'
>make: *** [vmlinux] Error 1
>
>Since the feature is to allow for a mixed IPV4 and IPV6 table, it
>seems sensible to make it depend on IPV6.
Acked-by: Patrick McHardy <kaber@trash.net>
>
>Cc: Patrick McHardy <kaber@trash.net>
>Cc: Pablo Neira Ayuso <pablo@netfilter.org>
>Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>
>diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
>index afe50c0f526f..c37467562fd0 100644
>--- a/net/netfilter/Kconfig
>+++ b/net/netfilter/Kconfig
>@@ -429,7 +429,7 @@ config NF_TABLES
> To compile it as a module, choose M here.
>
> config NF_TABLES_INET
>- depends on NF_TABLES
>+ depends on NF_TABLES && IPV6
> select NF_TABLES_IPV4
> select NF_TABLES_IPV6
> tristate "Netfilter nf_tables mixed IPv4/IPv6 tables support"
^ permalink raw reply
* Re: [PATCH 1/2] net/mlx4_core: clean up cq_res_start_move_to()
From: Paul Bolle @ 2014-01-14 11:23 UTC (permalink / raw)
To: Jack Morgenstein
Cc: Or Gerlitz, Rony Efraim, Hadar Hen Zion, David S. Miller, netdev,
linux-kernel
In-Reply-To: <20140114084752.1db64b21@jpm-OptiPlex-GX620>
On Tue, 2014-01-14 at 08:47 +0200, Jack Morgenstein wrote:
> On Tue, 07 Jan 2014 14:01:18 +0100
> Paul Bolle <pebolle@tiscali.nl> wrote:
>
> > + } else {
> > + /* state == RES_CQ_HW */
> > + if (r->com.state != RES_CQ_ALLOCATED)
>
> if (state != RES_CQ_HW || r->com.state != RES_CQ_ALLOCATED)
> to protect against any bad calls to this function
> (although I know that currently there are none).
So we end up with
} else if (state != RES_CQ_HW || r->com.state != RES_CQ_ALLOCATED) {
err = -EINVAL;
} else {
err = 0;
}
don't we? Which is fine with me, as GCC still is then able to correctly
analyze this function.
> This also preserves the behavior of the switch statement.
>
> > err = -EINVAL;
> > - }
> > + else
> > + err = 0;
> > + }
> >
> > - if (!err) {
> > - r->com.from_state = r->com.state;
> > - r->com.to_state = state;
> > - r->com.state = RES_CQ_BUSY;
> > - if (cq)
> > - *cq = r;
> > - }
> > + if (!err) {
> > + r->com.from_state = r->com.state;
> > + r->com.to_state = state;
> > + r->com.state = RES_CQ_BUSY;
>
> Please keep the "if" here. Protects against (future) bad calls.
>
> > + *cq = r;
> > }
There seems to be a school of thought that says it's better to trigger
an Oops if a programming error is made (in this case by passing a NULL
cq) then silently handle that (future) programming error and make
debugging harder. But, even it that school of thought really exists,
this is up to you. Besides, it's only a triviality I added to my
patches.
Thanks for the review! I hope to send in a v2 of my patches shortly.
Paul Bolle
^ permalink raw reply
* [PATCH] usbnet: Fix dma setup for fragmented packets that need a pad byte appended.
From: David Laight @ 2014-01-14 11:16 UTC (permalink / raw)
To: netdev
If the usbnet code appends a byte to a fragmented packet (in order to avoid
sending a bulk data message that is a multiple of the USB message size) then
the scatter-gather list isn't initialised correctly.
This causes a later panic in usb_hcd_map_urb_for_dma().
Basically when the code tries to access the final sg fragment the sg function
returns NULL because the 'end of sg list' market is set in the previous one.
Bug introduced in commit 60e453a940ac678565b6641d65f8c18541bb9f28
(USBNET: fix handling padding packet) and needs applying to all
kernels that contain this change (including 3.12).
Fix from Bjorn Mork.
Signed-off-by: David Laight <david.laight@aculab.com>
---
I think it is ok that the sg table's last element is never assigned to when
the packet isn't padded.
drivers/net/usb/usbnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 8494bb5..aba04f5 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1245,7 +1245,7 @@ static int build_dma_sg(const struct sk_buff *skb, struct urb *urb)
return -ENOMEM;
urb->num_sgs = num_sgs;
- sg_init_table(urb->sg, urb->num_sgs);
+ sg_init_table(urb->sg, urb->num_sgs + 1);
sg_set_buf(&urb->sg[s++], skb->data, skb_headlen(skb));
total_len += skb_headlen(skb);
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get (v3)
From: Andrey Wagin @ 2014-01-14 11:10 UTC (permalink / raw)
To: Andrew Vagin
Cc: Eric Dumazet, Florian Westphal, netfilter-devel, netfilter,
coreteam, netdev, LKML, vvs, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, Cyrill Gorcunov
In-Reply-To: <20140114105147.GA14538@paralelels.com>
>
> Eh, looks like this path is incomplete too:(
>
> I think we can't set a reference counter for objects which is allocated
> from a SLAB_DESTROY_BY_RCU cache. Look at the following backtrace.
>
> cpu1 cpu2
> ct = ____nf_conntrack_find()
> destroy_conntrack
> atomic_inc_not_zero(ct)
ct->ct_general.use is zero after destroy_conntrack(). Sorry for the noise.
^ permalink raw reply
* Re: [PATCH net-next v2.1 0/3] bonding: fix primary problem for bonding
From: Veaceslav Falico @ 2014-01-14 10:55 UTC (permalink / raw)
To: Ding Tianhong; +Cc: Jay Vosburgh, Netdev, David S. Miller
In-Reply-To: <52D5174D.4070807@huawei.com>
On Tue, Jan 14, 2014 at 06:54:05PM +0800, Ding Tianhong wrote:
>If the slave's name changed, and the bond params primary is exist,
>the bond should deal with the situation in two ways:
>
>1) If the slave was the primary slave yet, clean the primary slave
> and reselect active slave.
>2) If the slave's new name is as same as bond primary, set the slave
> as primary slave and reselect active slave.
>
>If the new primary is not matching any slave in the bond, the bond should
>record it to params, clean the primary slave and select a new active slave.
>
>Update bonding.txt for primary description.
>
>v2.1 Because there are too many indentions and useless verification, so rewrite
> the logic for updating the primary slave.
> Modify some comments for to clean the typos.
LOL. That's exactly what I was talking about in my previous email. A quick
fix that doesn't even address the issues.
Nacked-by: Veaceslav Falico <vfalico@redhat.com>
>
>Ding Tianhong (3):
> bonding: update the primary slave when changing slave's name
> bonding: clean the primary slave if there is no slave matching new
> primary
> bonding: update bonding.txt for primary description.
>
> Documentation/networking/bonding.txt | 3 ++-
> drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
> drivers/net/bonding/bond_options.c | 6 ++++++
> 3 files changed, 30 insertions(+), 3 deletions(-)
>
>--
>1.8.0
>
>
>
^ permalink raw reply
* [PATCH net-next v2.1 2/3] bonding: clean the primary slave if there is no slave matching new primary
From: Ding Tianhong @ 2014-01-14 10:54 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev
If the new primay is not matching any slave in the bond, the bond should
record it to params, clean the primary slave and select a new active slave.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_options.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 945a666..0ee0bfe 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -512,6 +512,12 @@ int bond_option_primary_set(struct bonding *bond, const char *primary)
}
}
+ if (bond->primary_slave) {
+ pr_info("%s: Setting primary slave to None.\n",
+ bond->dev->name);
+ bond->primary_slave = NULL;
+ bond_select_active_slave(bond);
+ }
strncpy(bond->params.primary, primary, IFNAMSIZ);
bond->params.primary[IFNAMSIZ - 1] = 0;
--
1.8.0
^ permalink raw reply related
* [PATCH net-next] bonding: handle slave's name change with primary_slave logic
From: Veaceslav Falico @ 2014-01-14 10:50 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Ding Tianhong, Jay Vosburgh, Andy Gospodarek
Currently, if a slave's name change, we just pass it by. However, if the
slave is a current primary_slave, then we end up with using a slave, whose
name != params.primary, for primary_slave. And vice-versa, if we don't have
a primary_slave but have params.primary set - we will not detected a new
primary_slave.
Fix this by catching the NETDEV_CHANGENAME event and setting primary_slave
accordingly. Also, if the primary_slave was changed, issue a reselection of
the active slave, cause the priorities have changed.
Reported-by: Ding Tianhong <dingtianhong@huawei.com>
CC: Ding Tianhong <dingtianhong@huawei.com>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e06c445..b16d7ec 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2860,9 +2860,26 @@ static int bond_slave_netdev_event(unsigned long event,
*/
break;
case NETDEV_CHANGENAME:
- /*
- * TODO: handle changing the primary's name
- */
+ /* we don't care if we don't have primary set */
+ if (!USES_PRIMARY(bond->params.mode) ||
+ !bond->params.primary[0])
+ break;
+
+ if (slave == bond->primary_slave) {
+ /* slave's name changed - he's no longer primary */
+ bond->primary_slave = NULL;
+ } else if (!strcmp(slave_dev->name, bond->params.primary)( {
+ /* we have a new primary slave */
+ bond->primary_slave = slave;
+ } else /* we didn't change primary - exit */
+ break;
+
+ pr_info("%s: Primary slave changed to %s, re-electing.\n",
+ bond->dev->name, bond->primary_slave ? slave_dev->name :
+ "none");
+ write_lock_bh(&bond->curr_slave_lock);
+ bond_select_active_slave(bond);
+ write_unlock_bh(&bond->curr_slave_lock);
break;
case NETDEV_FEAT_CHANGE:
bond_compute_features(bond);
--
1.8.4
^ permalink raw reply related
* [PATCH net-next v2.1 0/3] bonding: fix primary problem for bonding
From: Ding Tianhong @ 2014-01-14 10:54 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Netdev, David S. Miller
If the slave's name changed, and the bond params primary is exist,
the bond should deal with the situation in two ways:
1) If the slave was the primary slave yet, clean the primary slave
and reselect active slave.
2) If the slave's new name is as same as bond primary, set the slave
as primary slave and reselect active slave.
If the new primary is not matching any slave in the bond, the bond should
record it to params, clean the primary slave and select a new active slave.
Update bonding.txt for primary description.
v2.1 Because there are too many indentions and useless verification, so rewrite
the logic for updating the primary slave.
Modify some comments for to clean the typos.
Ding Tianhong (3):
bonding: update the primary slave when changing slave's name
bonding: clean the primary slave if there is no slave matching new
primary
bonding: update bonding.txt for primary description.
Documentation/networking/bonding.txt | 3 ++-
drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
drivers/net/bonding/bond_options.c | 6 ++++++
3 files changed, 30 insertions(+), 3 deletions(-)
--
1.8.0
^ permalink raw reply
* [PATCH net-next v2.1 3/3] bonding: update bonding.txt for primary description
From: Ding Tianhong @ 2014-01-14 10:54 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
Documentation/networking/bonding.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index a4d925e..5cdb229 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -657,7 +657,8 @@ primary
one slave is preferred over another, e.g., when one slave has
higher throughput than another.
- The primary option is only valid for active-backup mode.
+ The primary option is only valid for active-backup(1),
+ balance-tlb (5) and balance-alb (6) mode.
primary_reselect
--
1.8.0
^ permalink raw reply related
* [PATCH net-next v2.1 1/3] bonding: update the primary slave when changing slave's name
From: Ding Tianhong @ 2014-01-14 10:54 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, David S. Miller, Netdev
If the slave's name changed, and the bond params primary is exist,
the bond should deal with the situation in two ways:
1) If the slave was the primary slave yet, clean the primary slave
and reselect active slave.
2) If the slave's new name is as same as bond primary, set the slave
as primary slave and reselect active slave.
Thanks for Veaceslav's suggestion.
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e06c445..64e25d5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2860,9 +2860,29 @@ static int bond_slave_netdev_event(unsigned long event,
*/
break;
case NETDEV_CHANGENAME:
- /*
- * TODO: handle changing the primary's name
+ /* Handle changing the slave's name:
+ * 1) If the slave was primary slave,
+ * clean the primary slave and reselect
+ * active slave.
+ * 2) If the slave's new name is same as
+ * bond primary, set the slave as primary
+ * slave and reselect active slave.
*/
+ if (slave == bond->primary_slave ||
+ !strcmp(bond->params.primary, slave_dev->name)) {
+ if (bond->primary_slave) {
+ pr_info("%s: Setting primary slave to None.\n",
+ bond->dev->name);
+ bond->primary_slave = NULL;
+ } else {
+ pr_info("%s: Setting %s as primary slave.\n",
+ bond->dev->name, slave_dev->name);
+ bond->primary_slave = slave;
+ }
+ write_lock_bh(&bond->curr_slave_lock);
+ bond_select_active_slave(bond);
+ write_unlock_bh(&bond->curr_slave_lock);
+ }
break;
case NETDEV_FEAT_CHANGE:
bond_compute_features(bond);
--
1.8.0
^ permalink raw reply related
* Re: [PATCH RESEND net-next v2 1/3] bonding: update the primary slave when changing slave's name
From: Veaceslav Falico @ 2014-01-14 10:51 UTC (permalink / raw)
To: Ding Tianhong; +Cc: Jay Vosburgh, David S. Miller, Netdev
In-Reply-To: <52D4FE85.6050101@huawei.com>
On Tue, Jan 14, 2014 at 05:08:21PM +0800, Ding Tianhong wrote:
>If the slave's name changed, and the bond params primary is exist,
>the bond should deal with the situation in two ways:
>
>1) If the slave was the primary slave yet, clean the primary slave
> and reselect active slave.
>2) If the slave's new name is as same as bond primary, set the slave
> as primary slave and reselect active slave.
>
>Thanks for Veaceslav's suggestion.
>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bond_main.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index e06c445..64e25d5 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2860,9 +2860,29 @@ static int bond_slave_netdev_event(unsigned long event,
> */
> break;
> case NETDEV_CHANGENAME:
>- /*
>- * TODO: handle changing the primary's name
>+ /* Handle changing the slave's name:
>+ * 1) If the slave was primary slave,
>+ * clean the primary slave and reselect
>+ * active slave.
>+ * 2) If the slave's new name is same as
>+ * bond primary, set the slave as primary
>+ * slave and reselect active slave.
> */
>+ if (slave == bond->primary_slave ||
>+ !strcmp(bond->params.primary, slave_dev->name)) {
And if we're in a mode that doesn't use primary, but have the
params.primary set? Then we'll issue a bond_select_active_slave() in, say,
802.3ad mode.
In the past 24h I've nacked about 5 of your patchsets, with you keeping
'quickfixing' them, without getting your time to understand the issues, and
re-sending them for review.
I'm not willing to waste my time that uselessly, reviewing patchsets that
you randomly generate in the hope of getting it right. And given your
'good' history - with patchsets that cause regressions and bugs, with
reverts because of that, with those horrible, meaningless RCU transition
that is just plainly wrong and *really* hard to fix - I'm going to react as
Greg KH said in one of his presentations - NAK your patches and make them
by myself. It will take *a lot* lesser time from my side, and will
eventually make the code better.
Thanks for the report, I'll send a patch that fixes it soon.
Nacked-by: Veaceslav Falico <vfalico@redhat.com>
>+ if (bond->primary_slave) {
>+ pr_info("%s: Setting primary slave to None.\n",
>+ bond->dev->name);
>+ bond->primary_slave = NULL;
>+ } else {
>+ pr_info("%s: Setting %s as primary slave.\n",
>+ bond->dev->name, slave_dev->name);
>+ bond->primary_slave = slave;
>+ }
>+ write_lock_bh(&bond->curr_slave_lock);
>+ bond_select_active_slave(bond);
>+ write_unlock_bh(&bond->curr_slave_lock);
>+ }
> break;
> case NETDEV_FEAT_CHANGE:
> bond_compute_features(bond);
>--
>1.8.0
>
>
^ permalink raw reply
* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get (v3)
From: Andrew Vagin @ 2014-01-14 10:51 UTC (permalink / raw)
To: Eric Dumazet, Florian Westphal
Cc: Andrey Vagin, netfilter-devel, netfilter, coreteam, netdev,
linux-kernel, vvs, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, Cyrill Gorcunov
In-Reply-To: <1389558074.31367.187.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Jan 12, 2014 at 12:21:14PM -0800, Eric Dumazet wrote:
> On Sun, 2014-01-12 at 21:50 +0400, Andrey Vagin wrote:
> > Lets look at destroy_conntrack:
> >
> > hlist_nulls_del_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode);
> > ...
> > nf_conntrack_free(ct)
> > kmem_cache_free(net->ct.nf_conntrack_cachep, ct);
> >
> > net->ct.nf_conntrack_cachep is created with SLAB_DESTROY_BY_RCU.
> >
> > The hash is protected by rcu, so readers look up conntracks without
> > locks.
> > A conntrack is removed from the hash, but in this moment a few readers
> > still can use the conntrack. Then this conntrack is released and another
> > thread creates conntrack with the same address and the equal tuple.
> > After this a reader starts to validate the conntrack:
> > * It's not dying, because a new conntrack was created
> > * nf_ct_tuple_equal() returns true.
> ...
>
>
> > v2: move nf_ct_is_confirmed into the unlikely() annotation
> > v3: Eric suggested to fix refcnt, so that it becomes zero before adding
> > in a hash, but we can't find a way how to do that. Another way is to
> > interpret the confirm bit as part of a search key and check it in
> > ____nf_conntrack_find() too.
> >
> > Cc: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Florian Westphal <fw@strlen.de>
> > Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> > Cc: Patrick McHardy <kaber@trash.net>
> > Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> > Signed-off-by: Andrey Vagin <avagin@openvz.org>
> > ---
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
> Thanks Andrey !
>
Eh, looks like this path is incomplete too:(
I think we can't set a reference counter for objects which is allocated
from a SLAB_DESTROY_BY_RCU cache. Look at the following backtrace.
cpu1 cpu2
ct = ____nf_conntrack_find()
destroy_conntrack
atomic_inc_not_zero(ct)
__nf_conntrack_alloc
atomic_set(&ct->ct_general.use, 1);
if (!nf_ct_key_equal(h, tuple, zone))
nf_ct_put(ct);
destroy_conntrack(ct) !!!!
/* continues to use the conntrack */
Did I miss something again?
I think __nf_conntrack_alloc must use atomic_inc instead of
atomic_set. And we must be sure, that the first object from a new page is
zeroized.
I am talking about this, because after this patch a bug was triggered from
another place:
<2>[67096.759353] kernel BUG at net/netfilter/nf_conntrack_core.c:211!
<4>[67096.759371] invalid opcode: 0000 [#1] SMP
<4>[67096.759385] last sysfs file: /sys/devices/virtual/block/md0/md/metadata_version
<4>[67096.759414] CPU 2
<4>[67096.759422] Modules linked in: xt_comment sch_sfq cls_fw sch_htb pio_nfs pio_direct pfmt_raw pfmt_ploop1 ploop simfs xt_string xt_hashlimit xt_recent xt_length xt_hl xt_tcpmss xt_TCPMSS xt_multiport xt_limit xt_dscp vzevent coretemp cpufreq_ondemand acpi_cpufreq freq_table mperf 8021q garp stp llc ipt_REJECT iptable_filter iptable_mangle xt_NOTRACK iptable_raw iptable_nat ip_tables ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_state ip6table_filter ip6table_raw xt_MARK ip6table_mangle ip6_tables ext4 jbd2 tun ip_gre ipip vzethdev vznetdev vzrst nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 ipv6 vzcpt nf_conntrack vzdquota vzmon vziolimit vzdev tunnel4 nfsd nfs lockd fscache auth_rpcgss nfs_acl sunrpc tpm_tis tpm tpm_bios microcode serio_raw i2c_i801 sg iTCO_wdt iTCO_vendor_support e1000e
ext3 jbd mbcache raid1 sd_mod crc_t10dif ata_piix ahci pata_acpi ata_generic i915 drm_kms_helper drm i2c_algo_bit i2c_core video output dm_mirror dm_region_hash dm_log dm_mod [last unloaded: scsi_wait_scan]
<4>[67096.759801]
<4>[67096.759837] Pid: 498649, comm: atdd veid: 666 Tainted: G C --------------- 2.6.32-042stab084.18 #1 042stab084_18 /DQ45CB
<4>[67096.759932] RIP: 0010:[<ffffffffa03d99ac>] [<ffffffffa03d99ac>] destroy_conntrack+0x15c/0x190 [nf_conntrack]
<4>[67096.760032] RSP: 0000:ffff88001ae378b8 EFLAGS: 00010246
<4>[67096.760075] RAX: 0000000000000000 RBX: ffff8801a57ac928 RCX: 0000000000065000
<4>[67096.760123] RDX: 000000000000f603 RSI: 0000000000000006 RDI: ffff8801a57ac928
<4>[67096.760174] RBP: ffff88001ae378d8 R08: 0000000000000002 R09: ffff8802373b06e0
<4>[67096.760221] R10: 0000000000000001 R11: 0000000000000000 R12: ffff88023928c080
<4>[67096.760255] R13: ffff880237e8c000 R14: 0000000000000002 R15: 0000000000000002
<4>[67096.760255] FS: 0000000000000000(0000) GS:ffff880028300000(0063) knlGS:00000000b63afbb0
<4>[67096.760255] CS: 0010 DS: 002b ES: 002b CR0: 000000008005003b
<4>[67096.760255] CR2: 00000000b74f44c0 CR3: 00000000b89c6000 CR4: 00000000000007e0
<4>[67096.760255] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
<4>[67096.760255] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
<4>[67096.760255] Process atdd (pid: 498649, veid: 666, threadinfo ffff88001ae36000, task ffff88001deaa980)
<4>[67096.760255] Stack:
<4>[67096.760255] ffff88001ae378e8 ffff88001ae37988 ffff88023928c080 0000000000000003
<4>[67096.760255] <d> ffff88001ae378e8 ffffffff814844a7 ffff88001ae37908 ffffffffa03d9bb5
<4>[67096.760255] <d> ffff88012dcae580 ffff88023928c080 ffff88001ae379e8 ffffffffa03d9fb2
<4>[67096.760255] Call Trace:
<4>[67096.760255] [<ffffffff814844a7>] nf_conntrack_destroy+0x17/0x30
<4>[67096.760255] [<ffffffffa03d9bb5>] nf_conntrack_find_get+0x85/0x130 [nf_conntrack]
<4>[67096.760255] [<ffffffffa03d9fb2>] nf_conntrack_in+0x352/0xb60 [nf_conntrack]
<4>[67096.760255] [<ffffffffa048c771>] ipv4_conntrack_local+0x51/0x60 [nf_conntrack_ipv4]
<4>[67096.760255] [<ffffffff81484419>] nf_iterate+0x69/0xb0
<4>[67096.760255] [<ffffffff814b5b00>] ? dst_output+0x0/0x20
<4>[67096.760255] [<ffffffff814845d4>] nf_hook_slow+0x74/0x110
<4>[67096.760255] [<ffffffff814b5b00>] ? dst_output+0x0/0x20
<4>[67096.760255] [<ffffffff814b66d5>] raw_sendmsg+0x775/0x910
<4>[67096.760255] [<ffffffff8104c5a8>] ? flush_tlb_others_ipi+0x128/0x130
<4>[67096.760255] [<ffffffff8100bc4e>] ? apic_timer_interrupt+0xe/0x20
<4>[67096.760255] [<ffffffff8100bc4e>] ? apic_timer_interrupt+0xe/0x20
<4>[67096.760255] [<ffffffff814c136a>] inet_sendmsg+0x4a/0xb0
<4>[67096.760255] [<ffffffff81444e93>] ? sock_sendmsg+0x13/0x140
<4>[67096.760255] [<ffffffff81444f97>] sock_sendmsg+0x117/0x140
<4>[67096.760255] [<ffffffff8102e299>] ? native_smp_send_reschedule+0x49/0x60
<4>[67096.760255] [<ffffffff81519beb>] ? _spin_unlock_bh+0x1b/0x20
<4>[67096.760255] [<ffffffff8109d930>] ? autoremove_wake_function+0x0/0x40
<4>[67096.760255] [<ffffffff814960f0>] ? do_ip_setsockopt+0x90/0xd80
<4>[67096.760255] [<ffffffff8100bc4e>] ? apic_timer_interrupt+0xe/0x20
<4>[67096.760255] [<ffffffff8100bc4e>] ? apic_timer_interrupt+0xe/0x20
<4>[67096.760255] [<ffffffff814457c9>] sys_sendto+0x139/0x190
<4>[67096.760255] [<ffffffff810efa77>] ? audit_syscall_entry+0x1d7/0x200
<4>[67096.760255] [<ffffffff810ef7c5>] ? __audit_syscall_exit+0x265/0x290
<4>[67096.760255] [<ffffffff81474daf>] compat_sys_socketcall+0x13f/0x210
<4>[67096.760255] [<ffffffff8104dea3>] ia32_sysret+0x0/0x5
<4>[67096.760255] Code: 0b ab 0a e1 eb b7 f6 05 34 f8 00 e2 20 74 b7 80 3d f0 b0 00 00 00 74 ae 48 89 de 48 c7 c7 20 16 3e a0 31 c0 e8 05 ca 13 e1 eb 9b <0f> 0b eb fe f6 05 0b f8 00 e2 20 0f 84 db fe ff ff 80 3d eb b0
<1>[67096.760255] RIP [<ffffffffa03d99ac>] destroy_conntrack+0x15c/0x190 [nf_conntrack]
<4>[67096.760255] RSP <ffff88001ae378b8>
^ permalink raw reply
* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Steffen Klassert @ 2014-01-14 10:51 UTC (permalink / raw)
To: Fan Du; +Cc: davem, netdev
In-Reply-To: <52D5144B.605@windriver.com>
On Tue, Jan 14, 2014 at 06:41:15PM +0800, Fan Du wrote:
>
>
> On 2014年01月14日 18:34, Steffen Klassert wrote:
> >On Tue, Jan 14, 2014 at 06:17:26PM +0800, Fan Du wrote:
> >>On 2014年01月14日 18:09, Steffen Klassert wrote:
> >
> >No, I mean something like:
> >
> >sg_init_table(sg, nfrags + sglists)
> >
> >if (x->props.flags& XFRM_STATE_ESN) {
> > *seqhi = XFRM_SKB_CB(skb)->seq.input.hi;
> > sg_set_buf(sg + nfrags, seqhi, seqhi_len);
> >}
> >
>
> hehe, it's the same as the option this patch used.
No, you don't need to sg_unmark_end() before you can add
your entry and it is more obvious what happens here.
Also, I'm not absolutely sure whether the sg magic allows
what you did. Did you test with sg debugging enabled?
> Anyway, I will make it as you suggested in the next round review.
>
Thanks!
^ permalink raw reply
* Re: [PATCH v2] net: can: Disable flexcan driver build for big endian CPU on ARM
From: Arnd Bergmann @ 2014-01-14 10:44 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Wolfgang Grandegger, linux-can, netdev
> On 01/06/2014 02:21 PM, Guenter Roeck wrote:
> > Building arm:allmodconfig fails with
> >
> > flexcan.c: In function 'flexcan_read':
> > flexcan.c:243:2: error: implicit declaration of function 'in_be32'
> > flexcan.c: In function 'flexcan_write':
> > flexcan.c:248:2: error: implicit declaration of function 'out_be32'
> >
> > in_be32 and out_be32 do not (or no longer) exist for ARM targets.
> > Disable the build for ARM on big endian CPUs.
> >
> > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
>
> Applied to can-next.
Sorry, this patch was wrong.
There is no reason to disallow building the driver on big-endian
ARM kernels. Furthermore, the current behavior is actually broken
on little-endian PowerPC as well.
The choice of register accessor functions must purely depend
on the CPU architecture, not which endianess the CPU is running
on. Note that we nowadays allow both big-endian ARM and little-endian
PowerPC kernels.
With this patch applied, we will do the right thing in all four
combinations.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index aaed97b..320bef2 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -235,9 +235,12 @@ static const struct can_bittiming_const flexcan_bittiming_const = {
};
/*
- * Abstract off the read/write for arm versus ppc.
+ * Abstract off the read/write for arm versus ppc. This
+ * assumes that PPC uses big-endian registers and everything
+ * else uses little-endian registers, independent of CPU
+ * endianess.
*/
-#if defined(__BIG_ENDIAN)
+#if defined(CONFIG_PPC)
static inline u32 flexcan_read(void __iomem *addr)
{
return in_be32(addr);
^ permalink raw reply related
* Re: [PATCHv3 net-next 2/5] {IPv4,xfrm} Add ESN support for AH ingress part
From: Fan Du @ 2014-01-14 10:41 UTC (permalink / raw)
To: Steffen Klassert; +Cc: davem, netdev
In-Reply-To: <20140114103426.GJ31491@secunet.com>
On 2014年01月14日 18:34, Steffen Klassert wrote:
> On Tue, Jan 14, 2014 at 06:17:26PM +0800, Fan Du wrote:
>>
>>
>> On 2014年01月14日 18:09, Steffen Klassert wrote:
>>> On Tue, Jan 14, 2014 at 06:01:32PM +0800, Fan Du wrote:
>>>>
>>>>
>>>> On 2014年01月14日 17:54, Steffen Klassert wrote:
>>>>> On Tue, Jan 14, 2014 at 09:39:09AM +0800, Fan Du wrote:
>>>>>> @@ -381,7 +393,14 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
>>>>>> sg_init_table(sg, nfrags);
>>>>>> skb_to_sgvec(skb, sg, 0, skb->len);
>>>>>>
>>>>>> - ahash_request_set_crypt(req, sg, icv, skb->len);
>>>>>> + if (x->props.flags& XFRM_STATE_ESN) {
>>>>>> + sg_unmark_end(&sg[nfrags - 1]);
>>>>>> + /* Attach seqhi sg right after packet payload */
>>>>>> + *seqhi = htonl(XFRM_SKB_CB(skb)->seq.output.hi);
>>>>>
>>>>> This is ah_input(), so you should use the high bits of the input
>>>>> sequence number here. The ipv6 patch has the same problem.
>>>>
>>>> ok, I will fix this.
>>>>
>>>>>
>>>>>> + sg_init_table(seqhisg, sglists);
>>>>>
>>>>> Why do you add a separate SG table for this?
>>>>
>>>> It just initialize a single seqhisg, which is actually followed behind packet payload sg table.
>>>> initialized seqhisg actually mark itself as the end of sg list.
>>>>
>>>
>>> Why don't you just add this entry to the existing SG table?
>>>
>>
>> Do you mean scatterwalk_crypto_chain ?
>
> No, I mean something like:
>
> sg_init_table(sg, nfrags + sglists)
>
> if (x->props.flags& XFRM_STATE_ESN) {
> *seqhi = XFRM_SKB_CB(skb)->seq.input.hi;
> sg_set_buf(sg + nfrags, seqhi, seqhi_len);
> }
>
hehe, it's the same as the option this patch used.
Anyway, I will make it as you suggested in the next round review.
--
浮沉随浪只记今朝笑
--fan
^ permalink raw reply
* Re: [PATCH net-next v2 0/2] bonding: ensure that the TSO being set on bond master
From: Ding Tianhong @ 2014-01-14 10:38 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: Jay Vosburgh, Eric Dumazet, David S. Miller, Netdev
In-Reply-To: <20140114094741.GB20066@redhat.com>
On 2014/1/14 17:47, Veaceslav Falico wrote:
> On Tue, Jan 14, 2014 at 05:00:07PM +0800, Ding Tianhong wrote:
>> The commit b0ce3508(bonding: allow TSO being set on bonding master)
>> has make the TSO being set for bond dev, but in some situation, if
>> the slave did not have the NETIF_F_SG features, the bond master will
>> miss the TSO features in netdev_fix_features because the TSO is
>> depended on SG. So I have to add SG and TSO features on bond master
>> together.
>>
>> The function netdev_add_tso_features() was only be used for bonding,
>> so no need to export it in netdevice.h, remove it and add it to bonding.
>>
>> v2: If the slave hw did not support SG features, the SG should not
>> be forced open on master, otherwise error will occur, so modify it.
>> Some slave may support SG but not open it yet, so the bond master
>> could try to open it when adding the salve and make sure the TSO
>> could be open on master.
>
> So you're forcibly enabling SG on a slave? Usually SG is always enabled,
> unless there's a very specific reason for that - like a bug in hw/sw/fw,
> like performance improvement on specific use-cases etc. etc.
>
> So that's a wrong thing to do - to try to enable it, if it was disabled.
>
I don't think it is a wrong thing, just a slight changes of use, but your word
is reasonable, I will miss it.
> Nacked-by: Veaceslav Falico <vfalico@redhat.com>
>
>>
>> Ding Tianhong (2):
>> bonding: move the netdev_add_tso_features() to bonding
>> bonding: try to enable SG features when adding a new slave
>>
>> drivers/net/bonding/bond_main.c | 27 ++++++++++++++++++++++++++-
>> include/linux/netdevice.h | 10 ----------
>> 2 files changed, 26 insertions(+), 11 deletions(-)
>>
>> --
>> 1.8.0
>>
>>
>
>
^ 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