* [PATCH net-next] intel: pci_register_driver tail return
From: Stephen Hemminger @ 2013-09-05 17:22 UTC (permalink / raw)
To: Jeff Kirsher, David S. Miller; +Cc: netdev, e1000-devel
Remove unnecessary assignment, just return result of pci_register_driver
Glad to see that Intel does lots of code reuse :-)
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 5 +----
drivers/net/ethernet/intel/igb/igb_main.c | 4 +---
drivers/net/ethernet/intel/igbvf/netdev.c | 5 +----
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +---
4 files changed, 4 insertions(+), 14 deletions(-)
--- a/drivers/net/ethernet/intel/e1000e/netdev.c 2013-08-25 20:40:29.798995725 -0700
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c 2013-09-05 10:18:11.322040468 -0700
@@ -7054,13 +7054,10 @@ static struct pci_driver e1000_driver =
**/
static int __init e1000_init_module(void)
{
- int ret;
pr_info("Intel(R) PRO/1000 Network Driver - %s\n",
e1000e_driver_version);
pr_info("Copyright(c) 1999 - 2013 Intel Corporation.\n");
- ret = pci_register_driver(&e1000_driver);
-
- return ret;
+ return pci_register_driver(&e1000_driver);
}
module_init(e1000_init_module);
--- a/drivers/net/ethernet/intel/igb/igb_main.c 2013-09-05 08:10:16.494868469 -0700
+++ b/drivers/net/ethernet/intel/igb/igb_main.c 2013-09-05 09:52:11.914731506 -0700
@@ -680,7 +680,6 @@ struct net_device *igb_get_hw_dev(struct
**/
static int __init igb_init_module(void)
{
- int ret;
pr_info("%s - version %s\n",
igb_driver_string, igb_driver_version);
@@ -689,8 +688,7 @@ static int __init igb_init_module(void)
#ifdef CONFIG_IGB_DCA
dca_register_notify(&dca_notifier);
#endif
- ret = pci_register_driver(&igb_driver);
- return ret;
+ return pci_register_driver(&igb_driver);
}
module_init(igb_init_module);
--- a/drivers/net/ethernet/intel/igbvf/netdev.c 2013-08-10 10:36:10.293516206 -0700
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c 2013-09-05 09:55:11.644534238 -0700
@@ -2891,13 +2891,10 @@ static struct pci_driver igbvf_driver =
**/
static int __init igbvf_init_module(void)
{
- int ret;
pr_info("%s - version %s\n", igbvf_driver_string, igbvf_driver_version);
pr_info("%s\n", igbvf_copyright);
- ret = pci_register_driver(&igbvf_driver);
-
- return ret;
+ return pci_register_driver(&igbvf_driver);
}
module_init(igbvf_init_module);
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c 2013-09-05 08:10:16.494868469 -0700
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c 2013-09-05 09:53:27.937592738 -0700
@@ -3614,14 +3614,12 @@ static struct pci_driver ixgbevf_driver
**/
static int __init ixgbevf_init_module(void)
{
- int ret;
pr_info("%s - version %s\n", ixgbevf_driver_string,
ixgbevf_driver_version);
pr_info("%s\n", ixgbevf_copyright);
- ret = pci_register_driver(&ixgbevf_driver);
- return ret;
+ return pci_register_driver(&ixgbevf_driver);
}
module_init(ixgbevf_init_module);
^ permalink raw reply
* Re: [PATCH 2/2] ipv4 igmp: use del_timer_sync instead of del_timer in ip_mc_down
From: Salam Noureddine @ 2013-09-05 17:22 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <20130905084312.091f73b7@nehalam.linuxnetplumber.net>
On Thu, Sep 5, 2013 at 8:43 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Wed, 4 Sep 2013 23:43:24 -0700
> Salam Noureddine <noureddine@aristanetworks.com> wrote:
>
>> Delete timers using del_timer_sync in ip_mc_down. Otherwise, it is
>> possible for the timer to be the last to release its reference to the
>> in_device and since __in_dev_put doesn't destroy the in_device we
>> would end up leaking a reference to the net_device and see messages
>> like the following,
>>
>> unregister_netdevice: waiting for eth0 to become free. Usage count = 1
>>
>> Tested on linux-3.4.43.
>>
>> Signed-off-by: Salam Noureddine <noureddine@aristanetworks.com>
>
> Why not just call in_dev_put instead which just proper cleanup.
> It is less risky of deadlock than del_timer_sync.
I was wondering if there was a reason behind using __in_dev_put since
the multicast code
is the only user of that function. I can test using in_dev_put
instead. Should __in_dev_put
be removed altogether in that case?
Thanks,
Salam
^ permalink raw reply
* Re: [rfc] suspicious indentation in do_tcp_setsockopt
From: Neal Cardwell @ 2013-09-05 17:24 UTC (permalink / raw)
To: Joe Perches; +Cc: Dave Jones, Yuchung Cheng, Netdev
In-Reply-To: <1378356196.1787.74.camel@joe-AO722>
On Thu, Sep 5, 2013 at 12:43 AM, Joe Perches <joe@perches.com> wrote:
> (Adding Yuchung Cheng and Neal Cardwell as the
> author and acker of the patch)
>
> On Thu, 2013-09-05 at 00:20 -0400, Dave Jones wrote:
>> What's the intent here ?
>>
>> This ?
>
> I think the first is most likely.
Yes, exactly. The first version makes more sense. We only need to
check thin_dupack and potentially disable early retransmit if the
setsockopt successfully changes thin_dupack.
neal
>>
>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index b2f6c74..95544e4 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -2454,10 +2454,11 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>> case TCP_THIN_DUPACK:
>> if (val < 0 || val > 1)
>> err = -EINVAL;
>> - else
>> + else {
>> tp->thin_dupack = val;
>> if (tp->thin_dupack)
>> tcp_disable_early_retrans(tp);
>> + }
>> break;
>>
>> case TCP_REPAIR:
>>
>> Or this ...
>>
>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index b2f6c74..187c5a4 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -2456,8 +2456,9 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>> err = -EINVAL;
>> else
>> tp->thin_dupack = val;
>> - if (tp->thin_dupack)
>> - tcp_disable_early_retrans(tp);
>> +
>> + if (tp->thin_dupack)
>> + tcp_disable_early_retrans(tp);
>> break;
>>
>> case TCP_REPAIR:
>>
>>
>> I'll submit the right patch in the right form once I know what was intended.
>>
>> The former seems more 'correct' to me, but I'm unsure if that could break something.
>>
>> Dave
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" 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: [rfc] suspicious indentation in do_tcp_setsockopt
From: Yuchung Cheng @ 2013-09-05 17:34 UTC (permalink / raw)
To: Neal Cardwell; +Cc: Joe Perches, Dave Jones, Netdev
In-Reply-To: <CADVnQy=RuwyiBFmpLpoR4s=5HM70KiggitnK+tG6q2x2=3JaEw@mail.gmail.com>
On Thu, Sep 5, 2013 at 10:24 AM, Neal Cardwell <ncardwell@google.com> wrote:
> On Thu, Sep 5, 2013 at 12:43 AM, Joe Perches <joe@perches.com> wrote:
>> (Adding Yuchung Cheng and Neal Cardwell as the
>> author and acker of the patch)
>>
>> On Thu, 2013-09-05 at 00:20 -0400, Dave Jones wrote:
>>> What's the intent here ?
>>>
>>> This ?
>>
>> I think the first is most likely.
>
> Yes, exactly. The first version makes more sense. We only need to
> check thin_dupack and potentially disable early retransmit if the
> setsockopt successfully changes thin_dupack.
ack. first version is the intended one. thanks.
>
> neal
>
>
>>>
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index b2f6c74..95544e4 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -2454,10 +2454,11 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>>> case TCP_THIN_DUPACK:
>>> if (val < 0 || val > 1)
>>> err = -EINVAL;
>>> - else
>>> + else {
>>> tp->thin_dupack = val;
>>> if (tp->thin_dupack)
>>> tcp_disable_early_retrans(tp);
>>> + }
>>> break;
>>>
>>> case TCP_REPAIR:
>>>
>>> Or this ...
>>>
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index b2f6c74..187c5a4 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -2456,8 +2456,9 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>>> err = -EINVAL;
>>> else
>>> tp->thin_dupack = val;
>>> - if (tp->thin_dupack)
>>> - tcp_disable_early_retrans(tp);
>>> +
>>> + if (tp->thin_dupack)
>>> + tcp_disable_early_retrans(tp);
>>> break;
>>>
>>> case TCP_REPAIR:
>>>
>>>
>>> I'll submit the right patch in the right form once I know what was intended.
>>>
>>> The former seems more 'correct' to me, but I'm unsure if that could break something.
>>>
>>> Dave
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" 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: linux-next: Tree for Sep 5 (netfilter: xt_socket.c)
From: Randy Dunlap @ 2013-09-05 17:37 UTC (permalink / raw)
To: Stephen Rothwell
Cc: linux-next, linux-kernel, netdev@vger.kernel.org, netfilter-devel
In-Reply-To: <20130905193249.8b7c57afd5e39349ff5508bb@canb.auug.org.au>
On 09/05/13 02:32, Stephen Rothwell wrote:
> Hi all,
>
> Please do not add any code for v3.13 to your linux-next included branches
> until after v3.12-rc1 is released.
>
> Changes since 20130904:
>
on x86_64:
when CONFIG_IPV6=m
and CONFIG_NETFILTER_XT_MATCH_SOCKET=y:
net/built-in.o: In function `socket_mt6_v1_v2':
xt_socket.c:(.text+0x51b55): undefined reference to `udp6_lib_lookup'
net/built-in.o: In function `socket_mt_init':
xt_socket.c:(.init.text+0x1ef8): undefined reference to `nf_defrag_ipv6_enable'
--
~Randy
^ permalink raw reply
* Re: [PATCH] openvswitch: fix sw_flow_key alignment
From: Jesse Gross @ 2013-09-05 17:37 UTC (permalink / raw)
To: David Miller; +Cc: Andy Zhou, dev@openvswitch.org, netdev
In-Reply-To: <CAEP_g=-9ezK5MqiHKvKuFNn3SVoGBRy7p6V5JanKFFG86i8EDw@mail.gmail.com>
On Tue, Sep 3, 2013 at 3:06 PM, Jesse Gross <jesse@nicira.com> wrote:
> On Fri, Aug 30, 2013 at 9:16 PM, David Miller <davem@davemloft.net> wrote:
>> From: Jesse Gross <jesse@nicira.com>
>> Date: Fri, 30 Aug 2013 16:20:25 -0700
>>
>>> I looked through the struct definition and I think that the idea of
>>> manually padding as Geert did in his patch will be difficult to
>>> maintain over time (and actually there are a few that he missed) since
>>> there are a number of different structs/unions contained in there.
>>
>> You have to be mindful of the gaps and wasted space for performance
>> reasons anyways.
>
> Yes, although the approaches for performance and correctness are not
> necessarily the same. For example, we're taking about potentially
> packing the struct, in which case we would still have the same
> alignment needs even without any gaps. If the correctness is clearly
> handled (through an explicit align and build assert) then it's easier
> to pack/rearrange/whatever for performance since the whole thing isn't
> fragile.
I'd like to get this build failure fixed soon, so I'll just send out a
minimal fix that seems best to me in a couple of minutes.
^ permalink raw reply
* [PATCH] openvswitch: Fix alignment of struct sw_flow_key.
From: Jesse Gross @ 2013-09-05 17:41 UTC (permalink / raw)
To: David Miller
Cc: netdev, dev, Andy Zhou, Fengguang Wu, Geert Uytterhoeven,
Jesse Gross
sw_flow_key alignment was declared as " __aligned(__alignof__(long))".
However, this breaks on the m68k architecture where long is 32 bit in
size but 16 bit aligned by default. This aligns to 8 bytes to ensure
that long is always covered without reducing native alignment. It also
adds an additional build check to catch any reduction in alignment.
CC: Andy Zhou <azhou@nicira.com>
Reported-by: Fengguang Wu <fengguan.wu@intel.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
net/openvswitch/flow.c | 1 +
net/openvswitch/flow.h | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index ad1aeeb..fb36f85 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -1981,6 +1981,7 @@ nla_put_failure:
* Returns zero if successful or a negative error code. */
int ovs_flow_init(void)
{
+ BUILD_BUG_ON(__alignof__(struct sw_flow_key) % __alignof__(long));
BUILD_BUG_ON(sizeof(struct sw_flow_key) % sizeof(long));
flow_cache = kmem_cache_create("sw_flow", sizeof(struct sw_flow), 0,
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index b65f885..3dc3b52 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -125,7 +125,7 @@ struct sw_flow_key {
} nd;
} ipv6;
};
-} __aligned(__alignof__(long));
+} __aligned(8); /* 8 byte alignment ensures this can be accessed as a long */
struct sw_flow {
struct rcu_head rcu;
--
1.8.1.2
^ permalink raw reply related
* Re: Intel igb module command line configuration in kernel sources
From: Ben Hutchings @ 2013-09-05 14:23 UTC (permalink / raw)
To: Andrew Davidoff; +Cc: netdev
In-Reply-To: <CAJLXCZRKdz-w8uR3ejzHrBot7OAyVJOWwzA0NszkZL16eFPTbQ@mail.gmail.com>
On Wed, 2013-09-04 at 22:51 -0400, Andrew Davidoff wrote:
> Hi,
>
> I apologize that this is a user-type question but linux-net seems to
> have gone away, and I cannot find a more appropriate networking
> related mailing list.
>
> Why are the Intel igb module configuration parameters, usually found
> in igb_param.c and part of the source from Intel, not included in the
> igb sources distributed with the linux kernel? I cannot find them in
> the kernel's igb source, modinfo doesn't list them, and if I try to
> use them as documented in the igb readme, I get errors because they
> are unrecognized (ex: RSS). Am I overlooking an alternate
> configuration method?
I don't know what you're specifically interested in, but a lot of this
should be configurable using ethtool.
Module parameters for device-specific settings are deprecated.
Ben.
--
Ben Hutchings, Staff 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
* [PATCH] Add missing braces to do_tcp_setsockopt
From: Dave Jones @ 2013-09-05 17:43 UTC (permalink / raw)
To: netdev; +Cc: Neal Cardwell, Yuchung Cheng
Signed-off-by: Dave Jones <davej@fedoraproject.org>
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b2f6c74..95544e4 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2454,10 +2454,11 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
case TCP_THIN_DUPACK:
if (val < 0 || val > 1)
err = -EINVAL;
- else
+ else {
tp->thin_dupack = val;
if (tp->thin_dupack)
tcp_disable_early_retrans(tp);
+ }
break;
case TCP_REPAIR:
^ permalink raw reply related
* Re: [net-next v4 7/8] i40e: sysfs and debugfs interfaces
From: Nelson, Shannon @ 2013-09-05 17:53 UTC (permalink / raw)
To: Stephen Hemminger, Kirsher, Jeffrey T
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Brandeburg, Jesse, gospo@redhat.com, davem@davemloft.net,
sassmann@redhat.com
In-Reply-To: <20130904173759.7c2eec52@nehalam.linuxnetplumber.net>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, September 04, 2013 5:38 PM
[...]
> Also, anything in sysfs is device specific and you really need to make
> a strong case for why your device is special and needs an exception.
> Other devices will have hardware switches and doing something through
> sysfs is going to create a pain for any controller application.
>
> I vote against including the sysfs VEB stuff because it will become
> a lifetime ABI.
If we simply remove the VEB attributes (cvlan, mode, seid, svlan) but keep the model structure and the VSI attributes, will that satisfy your vote, or are you suggesting that we should drop the whole sysfs model that we implemented?
At this point after some discussion internally, we think it would be better to simply remove the whole sysfs module - it is an optional section with no direct operational requirement. The intent was to get something started that looked useful, but perhaps we were a little premature in presenting this model for these new switch offload capabilities.
As Dave rightly pointed out, we may want to bring this back in the future, but I think that we have more work to do with the community in proposing and designing a switching model, if it really is even needed. That's not an over-night thing, and we shouldn't be trying to patch up something that has fundamental concerns. Also, bringing it back later when it isn't obscured by the rest of the driver might be more successful in getting community input.
sln
------------------------------------------------------------------------------
Learn the latest--Visual Studio 2012, SharePoint 2013, SQL 2012, more!
Discover the easy way to master current and previous Microsoft technologies
and advance your career. Get an incredible 1,500+ hours of step-by-step
tutorial videos with LearnDevNow. Subscribe today and save!
http://pubads.g.doubleclick.net/gampad/clk?id=58041391&iu=/4140/ostg.clktrk
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] Add missing braces to do_tcp_setsockopt
From: Neal Cardwell @ 2013-09-05 17:55 UTC (permalink / raw)
To: Dave Jones; +Cc: Netdev, Yuchung Cheng
In-Reply-To: <20130905174333.GA3021@redhat.com>
On Thu, Sep 5, 2013 at 1:43 PM, Dave Jones <davej@redhat.com> wrote:
> Signed-off-by: Dave Jones <davej@fedoraproject.org>
Acked-by: Neal Cardwell <ncardwell@google.com>
neal
^ permalink raw reply
* Re: [PATCH] Add missing braces to do_tcp_setsockopt
From: Dave Jones @ 2013-09-05 17:58 UTC (permalink / raw)
To: Neal Cardwell; +Cc: Netdev, Yuchung Cheng
In-Reply-To: <CADVnQy=_pXCdJH_qVyz6MOr-2Qo3FahYK4itThmaa2YKJm2PgA@mail.gmail.com>
On Thu, Sep 05, 2013 at 01:55:48PM -0400, Neal Cardwell wrote:
> On Thu, Sep 5, 2013 at 1:43 PM, Dave Jones <davej@redhat.com> wrote:
> > Signed-off-by: Dave Jones <davej@fedoraproject.org>
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
>
> neal
Crap, I forgot the
introduced in eed530b6c67624db3f2cf477bac7c4d005d8f7ba ("tcp: early retransmit")
Dave, shall I resend ?
Dave
^ permalink raw reply
* RE: [net-next v4 1/7] i40e: main driver core
From: Nelson, Shannon @ 2013-09-05 17:59 UTC (permalink / raw)
To: Stephen Hemminger, Kirsher, Jeffrey T
Cc: davem@davemloft.net, Brandeburg, Jesse, netdev@vger.kernel.org,
gospo@redhat.com, sassmann@redhat.com, Waskiewicz Jr, Peter P,
e1000-devel@lists.sourceforge.net
In-Reply-To: <20130905092633.253896e3@nehalam.linuxnetplumber.net>
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, September 05, 2013 9:27 AM
>
> More review comments, these are non-blocking, and can be fixed later.
Thanks, Stephen, we'll roll these updates in soon.
sln
>
> Minor
> -----
> * style
> not a fan of doing own error codes and handling (ie. I40E_SUCCESS)
> which probably is side effect of OS abstraction.
>
> * extra parens aren't needed on return
> return (budget > 0);
>
> * i40e_config_netdev
> etherdev_size = (sizeof(struct i40e_netdev_priv));
> Extra paren, only used once, unsigned vs signed, just
> use sizeof()
> strlcpy(netdev->name, "eth%d", IFNAMSIZ-1);
> That is already done by alloc_etherdev
> memcpy(netdev->dev_addr, mac_addr, netdev->addr_len);
> netdev->addr_len is ETH_ALEN, mac_addr is ETH_ALEN
> just use constant size copy.
> * i40e_init_module
> ret = pci_register_driver(&i40e_driver);
> return ret;
> assignment is unneeded, just do return pci_register_driver
>
> * service timer
> since it is 1 HZ, you should use round_jiffies to align
> on clock boundary to save power.
>
> * Why is Kbuild file used instead of Makefile like other drivers?
>
> * Ethtool
> Need to use ethtool_cmd_speed_set othewise only lower bit set.
>
> * Extension I40E_DEBUG_USER is non-standard
>
> * What is is point of using snprintf() in i40e_get_drvinfo
> snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
> "%s", i40e_fw_version_str(&pf->hw));
> Already using strlcpy() for other fields.
>
> * Ethtool ops can be const
>
> * isn't vsi->netdev = NULL needed here.
> s32 i40e_vsi_release(struct i40e_vsi *vsi)
> if (vsi->netdev_registered) {
> vsi->netdev_registered = false;
> if (vsi->netdev) {
> /* results in a call to i40e_close() */
> unregister_netdev(vsi->netdev);
> free_netdev(vsi->netdev);
> >>>> vsi->netdev = NULL;
>
>
> Enhancement
> -----------
> * Support Byte Queue Limits
> - might be tougher with VBE
>
^ permalink raw reply
* Re: [RFC Patch net-next] ipv6: do not allow ipv6 module to be removed
From: David Miller @ 2013-09-05 18:06 UTC (permalink / raw)
To: amwang; +Cc: netdev, yoshfuji, stephen
In-Reply-To: <1378285970-21404-1-git-send-email-amwang@redhat.com>
From: Cong Wang <amwang@redhat.com>
Date: Wed, 4 Sep 2013 17:12:50 +0800
> There was some bug report on ipv6 module removal path before.
> Also, as Stephen pointed out, after vxlan module gets ipv6 support,
> the ipv6 stub it used is not safe against this module removal either.
> So, let's just remove inet6_exit() so that ipv6 module will not be
> able to be unloaded.
>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>
I think we should make an effort to make ipv6 unloadable. If you
believe little effort is being put into this now, even less will
be expended if I apply a patch like this one.
And this patch has a lot of problems, you aren't removing all of
the functions which will be completely unused.
^ permalink raw reply
* Re: [PATCH] openvswitch: Fix alignment of struct sw_flow_key.
From: David Miller @ 2013-09-05 18:17 UTC (permalink / raw)
To: jesse; +Cc: netdev, dev, azhou, fengguan.wu, geert
In-Reply-To: <1378402887-17331-1-git-send-email-jesse@nicira.com>
From: Jesse Gross <jesse@nicira.com>
Date: Thu, 5 Sep 2013 10:41:27 -0700
> -} __aligned(__alignof__(long));
> +} __aligned(8); /* 8 byte alignment ensures this can be accessed as a long */
This kind of stuff drives me crazy.
If the issue is the type, therefore at least use an expression that
mentions the type explicitly. And mention the actual type that
matters. "long" isn't it.
These half-hearted attempts to fix this problem are just papering
around the issue and do not address the real issue directly.
I don't want to apply changes like this.
You don't want "8 byte alignment", you want the alignment necessary
for the types contained in the structure to be accessed without
generating exceptions. You might also want the "quickest" alignment,
and there are ways to express that as well.
This structure doesn't even contain a "long". "long" isn't even
64-bit on some architectures, and you certainly have 64-bit types in
this thing (__be64).
Let's start from the beginning, what are you actually trying to
accomplish here that isn't handled automatically by the compiler
already?
Is it "proper" alignment? That is handled automatically by the
compiler. Is it "fast" alignment? In what context and in what ways
is that important in this specific case?
^ permalink raw reply
* Re: [PATCH] Add missing braces to do_tcp_setsockopt
From: David Miller @ 2013-09-05 18:18 UTC (permalink / raw)
To: davej; +Cc: ncardwell, netdev, ycheng
In-Reply-To: <20130905175847.GB5682@redhat.com>
From: Dave Jones <davej@redhat.com>
Date: Thu, 5 Sep 2013 13:58:47 -0400
> On Thu, Sep 05, 2013 at 01:55:48PM -0400, Neal Cardwell wrote:
> > On Thu, Sep 5, 2013 at 1:43 PM, Dave Jones <davej@redhat.com> wrote:
> > > Signed-off-by: Dave Jones <davej@fedoraproject.org>
> >
> > Acked-by: Neal Cardwell <ncardwell@google.com>
> >
> > neal
>
> Crap, I forgot the
>
> introduced in eed530b6c67624db3f2cf477bac7c4d005d8f7ba ("tcp: early retransmit")
>
> Dave, shall I resend ?
I'll take care of it.
^ permalink raw reply
* Re: Add missing braces in bnx2x:bnx2x_link_initialize
From: David Miller @ 2013-09-05 18:21 UTC (permalink / raw)
To: davej; +Cc: netdev, eilong
In-Reply-To: <20130905034658.GA12717@redhat.com>
David, I'm going to apply your missing braces patches, but I'm really irritated
that I've told you at least 5 times to put proper subsystem prefixes into
your subject lines.
For this I'd use "bnx2x: ", for the TCP patch I'd use "tcp: " and for the
CAIF change I'd use "caif: "
Please get into the habit of doing this, or I'm going to push back on you
to correct this instead of fixing it up automatically for you every time,
since the latter really is not scalable.
^ permalink raw reply
* Re: [PATCH] ethernet/arc/arc_emac: optimize the Tx/Tx-reclaim paths a bit
From: David Miller @ 2013-09-05 18:24 UTC (permalink / raw)
To: Vineet.Gupta1; +Cc: netdev, Alexey.Brodkin, romieu, linux-kernel, arc-linux-dev
In-Reply-To: <1378299791-24598-1-git-send-email-vgupta@synopsys.com>
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Date: Wed, 4 Sep 2013 18:33:11 +0530
> This came out of staring at code due to recent performance fix.
>
> * TX BD reclaim can call netif_wake_queue() once, outside the loop if
> one/more BDs were freed, NO need to do this each iteration.
>
> * TX need not look at next BD to stop the netif queue. It rather be done
> in the next tx call, when it actually fails as the queue seldom gets
> full but the check nevertheless needs to be done for each packet Tx.
> Profiled this under heavy traffic (big tar file cp, LMBench betworking
> tests) and saw not a single hit to that code.
>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
You should keep the check in the transmit queueing code as a BUG check,
almost every driver has code of the form (using NIU as an example):
if (niu_tx_avail(rp) <= (skb_shinfo(skb)->nr_frags + 1)) {
netif_tx_stop_queue(txq);
dev_err(np->device, "%s: BUG! Tx ring full when queue awake!\n", dev->name);
rp->tx_errors++;
return NETDEV_TX_BUSY;
}
and arc_emac should too.
Otherwise queue management bugs are incredibly hard to diagnose.
I'm not applying this patch.
^ permalink raw reply
* Re: [PATCH] ethernet/arc/arc_emac: Fix huge delays in large file copies
From: David Miller @ 2013-09-05 18:25 UTC (permalink / raw)
To: Vineet.Gupta1; +Cc: netdev, Alexey.Brodkin, linux-kernel, arc-linux-dev
In-Reply-To: <1378295235-18928-1-git-send-email-vgupta@synopsys.com>
From: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Date: Wed, 4 Sep 2013 17:17:15 +0530
> copying large files to a NFS mounted host was taking absurdly large
> time.
>
> Turns out that TX BD reclaim had a sublte bug.
>
> Loop starts off from @txbd_dirty cursor and stops when it hits a BD
> still in use by controller. However when it stops it needs to keep the
> cursor at that very BD to resume scanning in next iteration. However it
> was erroneously incrementing the cursor, causing the next scan(s) to
> fail too, unless the BD chain was completely drained out.
>
> [ARCLinux]$ ls -l -sh /disk/log.txt
> 17976 -rw-r--r-- 1 root root 17.5M Sep /disk/log.txt
>
> ========== Before =====================
> [ARCLinux]$ time cp /disk/log.txt /mnt/.
> real 31m 7.95s
> user 0m 0.00s
> sys 0m 0.10s
>
> ========== After =====================
> [ARCLinux]$ time cp /disk/log.txt /mnt/.
> real 0m 24.33s
> user 0m 0.00s
> sys 0m 0.19s
>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Applied.
^ permalink raw reply
* Re: [PATCH] icplus: Use netif_running to determine device state
From: David Miller @ 2013-09-05 18:27 UTC (permalink / raw)
To: jdmason; +Cc: netdev, romieu, sorbica
In-Reply-To: <1378319161-18850-1-git-send-email-jdmason@kudzu.us>
From: Jon Mason <jdmason@kudzu.us>
Date: Wed, 4 Sep 2013 11:26:01 -0700
> Remove the __LINK_STATE_START check to verify the device is running, in
> favor of netif_running(). netif_running() performs the same check of
> __LINK_STATE_START, so the code should behave the same.
>
> Signed-off-by: Jon Mason <jdmason@kudzu.us>
Applied, thanks.
^ permalink raw reply
* Re: Add missing braces in bnx2x:bnx2x_link_initialize
From: Dave Jones @ 2013-09-05 18:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eilong
In-Reply-To: <20130905.142117.1916319107625842387.davem@davemloft.net>
On Thu, Sep 05, 2013 at 02:21:17PM -0400, David Miller wrote:
>
> David, I'm going to apply your missing braces patches, but I'm really irritated
> that I've told you at least 5 times to put proper subsystem prefixes into
> your subject lines.
>
> For this I'd use "bnx2x: ", for the TCP patch I'd use "tcp: " and for the
> CAIF change I'd use "caif: "
I knew I'd forget something..
> Please get into the habit of doing this, or I'm going to push back on you
> to correct this instead of fixing it up automatically for you every time,
> since the latter really is not scalable.
Sure thing.
thanks,
Dave
^ permalink raw reply
* Re: [E1000-devel] [net-next v4 7/8] i40e: sysfs and debugfs interfaces
From: Stephen Hemminger @ 2013-09-05 18:32 UTC (permalink / raw)
To: Nelson, Shannon
Cc: Kirsher, Jeffrey T, e1000-devel@lists.sourceforge.net,
netdev@vger.kernel.org, Brandeburg, Jesse, gospo@redhat.com,
davem@davemloft.net, sassmann@redhat.com
In-Reply-To: <FC41C24E35F18A40888AACA1A36F3E416C6249AD@FMSMSX102.amr.corp.intel.com>
On Thu, 5 Sep 2013 17:53:38 +0000
"Nelson, Shannon" <shannon.nelson@intel.com> wrote:
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Wednesday, September 04, 2013 5:38 PM
>
> [...]
>
> > Also, anything in sysfs is device specific and you really need to make
> > a strong case for why your device is special and needs an exception.
> > Other devices will have hardware switches and doing something through
> > sysfs is going to create a pain for any controller application.
> >
> > I vote against including the sysfs VEB stuff because it will become
> > a lifetime ABI.
>
> If we simply remove the VEB attributes (cvlan, mode, seid, svlan) but keep the model structure and the VSI attributes, will that satisfy your vote, or are you suggesting that we should drop the whole sysfs model that we implemented?
>
> At this point after some discussion internally, we think it would be better to simply remove the whole sysfs module - it is an optional section with no direct operational requirement. The intent was to get something started that looked useful, but perhaps we were a little premature in presenting this model for these new switch offload capabilities.
>
> As Dave rightly pointed out, we may want to bring this back in the future, but I think that we have more work to do with the community in proposing and designing a switching model, if it really is even needed. That's not an over-night thing, and we shouldn't be trying to patch up something that has fundamental concerns. Also, bringing it back later when it isn't obscured by the rest of the driver might be more successful in getting community input.
>
> sln
>
>
IMHO attributes are a nice way of handling the VSI attributes since they
seem hardware specific. Not sure how to do the right thing with switching.
Should it look like Macvlan, bridge, VXLAN, or something else.
^ permalink raw reply
* Re: [PATCH] openvswitch: Fix alignment of struct sw_flow_key.
From: Jesse Gross @ 2013-09-05 18:36 UTC (permalink / raw)
To: David Miller
Cc: netdev, dev@openvswitch.org, Andy Zhou, fengguan.wu,
Geert Uytterhoeven
In-Reply-To: <20130905.141751.945639989861997124.davem@davemloft.net>
On Thu, Sep 5, 2013 at 11:17 AM, David Miller <davem@davemloft.net> wrote:
> From: Jesse Gross <jesse@nicira.com>
> Date: Thu, 5 Sep 2013 10:41:27 -0700
>
>> -} __aligned(__alignof__(long));
>> +} __aligned(8); /* 8 byte alignment ensures this can be accessed as a long */
>
> This kind of stuff drives me crazy.
>
> If the issue is the type, therefore at least use an expression that
> mentions the type explicitly. And mention the actual type that
> matters. "long" isn't it.
'long' actually is the real type here.
When doing comparisons, this structure is being accessed as a byte
array in 'long' sized chunks, not by its members. Therefore, the
compiler's alignment does not necessarily correspond to anything for
this purpose. It could be a struct full of u16's and we would still
want to access it in chunks of 'long'.
To completely honest, I think the correct alignment should be
sizeof(long) because I know that 'long' is not always 8 bytes on all
architectures. However, you made the point before that this could
break the alignment of the 64-bit values on architectures where 'long'
is 32 bits wide, so 8 bytes is the generic solution.
^ permalink raw reply
* Re: linux-next: Tree for Sep 5 (netfilter: xt_socket.c)
From: David Miller @ 2013-09-05 18:38 UTC (permalink / raw)
To: rdunlap; +Cc: sfr, linux-next, linux-kernel, netdev, netfilter-devel
In-Reply-To: <5228C140.1010504@infradead.org>
From: Randy Dunlap <rdunlap@infradead.org>
Date: Thu, 05 Sep 2013 10:37:04 -0700
> On 09/05/13 02:32, Stephen Rothwell wrote:
>> Hi all,
>>
>> Please do not add any code for v3.13 to your linux-next included branches
>> until after v3.12-rc1 is released.
>>
>> Changes since 20130904:
>>
>
> on x86_64:
>
> when CONFIG_IPV6=m
> and CONFIG_NETFILTER_XT_MATCH_SOCKET=y:
>
> net/built-in.o: In function `socket_mt6_v1_v2':
> xt_socket.c:(.text+0x51b55): undefined reference to `udp6_lib_lookup'
> net/built-in.o: In function `socket_mt_init':
> xt_socket.c:(.init.text+0x1ef8): undefined reference to `nf_defrag_ipv6_enable'
I just commited the following to fix this:
--------------------
[PATCH] netfilter: Fix build errors with xt_socket.c
As reported by Randy Dunlap:
====================
when CONFIG_IPV6=m
and CONFIG_NETFILTER_XT_MATCH_SOCKET=y:
net/built-in.o: In function `socket_mt6_v1_v2':
xt_socket.c:(.text+0x51b55): undefined reference to `udp6_lib_lookup'
net/built-in.o: In function `socket_mt_init':
xt_socket.c:(.init.text+0x1ef8): undefined reference to `nf_defrag_ipv6_enable'
====================
Like several other modules under net/netfilter/ we have to
have a dependency "IPV6 disabled or set compatibly with this
module" clause.
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/netfilter/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 62a171a..6e839b6 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -1175,6 +1175,7 @@ config NETFILTER_XT_MATCH_SOCKET
depends on NETFILTER_XTABLES
depends on NETFILTER_ADVANCED
depends on !NF_CONNTRACK || NF_CONNTRACK
+ depends on (IPV6 || IPV6=n)
select NF_DEFRAG_IPV4
select NF_DEFRAG_IPV6 if IP6_NF_IPTABLES
help
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] openvswitch: Fix alignment of struct sw_flow_key.
From: David Miller @ 2013-09-05 18:40 UTC (permalink / raw)
To: jesse-l0M0P4e3n4LQT0dZR+AlfA
Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
fengguan.wu-ral2JQCrhuEAvxtiuMwx3w, geert-Td1EMuHUCqxL1ZNQvxDV9g
In-Reply-To: <CAEP_g=8gcCtkv=no=HkVmS+NDWPM-Uu-SVyR+vb-YQ=7TWJsXA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
Date: Thu, 5 Sep 2013 11:36:19 -0700
> On Thu, Sep 5, 2013 at 11:17 AM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
>> From: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
>> Date: Thu, 5 Sep 2013 10:41:27 -0700
>>
>>> -} __aligned(__alignof__(long));
>>> +} __aligned(8); /* 8 byte alignment ensures this can be accessed as a long */
>>
>> This kind of stuff drives me crazy.
>>
>> If the issue is the type, therefore at least use an expression that
>> mentions the type explicitly. And mention the actual type that
>> matters. "long" isn't it.
>
> 'long' actually is the real type here.
>
> When doing comparisons, this structure is being accessed as a byte
> array in 'long' sized chunks, not by its members. Therefore, the
> compiler's alignment does not necessarily correspond to anything for
> this purpose. It could be a struct full of u16's and we would still
> want to access it in chunks of 'long'.
>
> To completely honest, I think the correct alignment should be
> sizeof(long) because I know that 'long' is not always 8 bytes on all
> architectures. However, you made the point before that this could
> break the alignment of the 64-bit values on architectures where 'long'
> is 32 bits wide, so 8 bytes is the generic solution.
Look at net/core/flow.c:flow_key_compare().
And then we annotate struct flowi with
} __attribute__((__aligned__(BITS_PER_LONG/8)));
Don't reinvent the wheel, either mimick how existing code does
this kind of thing or provide a justification for doing it differently
and update the existing cases to match and be consistent.
^ 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