* Re: linux-next: build failure after merge of the net tree
From: David Miller @ 2010-02-05 6:32 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-next, linux-kernel, nicholasx.d.nunley
In-Reply-To: <20100205165059.0b5d5363.sfr@canb.auug.org.au>
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri, 5 Feb 2010 16:50:59 +1100
> Hi Dave,
>
> After merging the scsi-post-merge tree, today's linux-next build (powerpc
> allyesconfig and i386 defconfig) failed like this:
>
> drivers/net/e1000e/built-in.o: In function `e1000_has_link':
> (.opd+0x1d58): multiple definition of `e1000_has_link'
> drivers/net/e1000/built-in.o:(.opd+0x588): first defined here
>
> Caused by commit b548192acaebcb05d6a87d1e94f19835b1a18a8b ("e1000: Report
> link status in ethtool when interface is down").
>
> I have reverted that commit for today.
Damn namespace pollution, this is one of a zillion reasons
I prefer drivers are written in one sorce file, then people
can name their functions however they like and they're all
marked static so it doesn't cause problems like this.
I've fixed it up as follows in net-next-2.6
Thanks!
e1000e: Fix namespace conflicts wrt. e1000_has_link
Reported by Stephen Rothwell.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/e1000e/e1000.h | 2 +-
drivers/net/e1000e/ethtool.c | 2 +-
drivers/net/e1000e/netdev.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/e1000e/e1000.h b/drivers/net/e1000e/e1000.h
index 318bdb2..c2ec095 100644
--- a/drivers/net/e1000e/e1000.h
+++ b/drivers/net/e1000e/e1000.h
@@ -459,7 +459,7 @@ extern int e1000e_setup_tx_resources(struct e1000_adapter *adapter);
extern void e1000e_free_rx_resources(struct e1000_adapter *adapter);
extern void e1000e_free_tx_resources(struct e1000_adapter *adapter);
extern void e1000e_update_stats(struct e1000_adapter *adapter);
-extern bool e1000_has_link(struct e1000_adapter *adapter);
+extern bool e1000e_has_link(struct e1000_adapter *adapter);
extern void e1000e_set_interrupt_capability(struct e1000_adapter *adapter);
extern void e1000e_reset_interrupt_capability(struct e1000_adapter *adapter);
diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c
index 0aa50c2..b33e3cb 100644
--- a/drivers/net/e1000e/ethtool.c
+++ b/drivers/net/e1000e/ethtool.c
@@ -202,7 +202,7 @@ static u32 e1000_get_link(struct net_device *netdev)
if (!netif_carrier_ok(netdev))
mac->get_link_status = 1;
- return e1000_has_link(adapter);
+ return e1000e_has_link(adapter);
}
static int e1000_set_spd_dplx(struct e1000_adapter *adapter, u16 spddplx)
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 14a80f8..ffa37c6 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -3472,7 +3472,7 @@ static void e1000_print_link_info(struct e1000_adapter *adapter)
((ctrl & E1000_CTRL_TFCE) ? "TX" : "None" )));
}
-bool e1000_has_link(struct e1000_adapter *adapter)
+bool e1000e_has_link(struct e1000_adapter *adapter)
{
struct e1000_hw *hw = &adapter->hw;
bool link_active = 0;
@@ -3553,7 +3553,7 @@ static void e1000_watchdog_task(struct work_struct *work)
u32 link, tctl;
int tx_pending = 0;
- link = e1000_has_link(adapter);
+ link = e1000e_has_link(adapter);
if ((netif_carrier_ok(netdev)) && link) {
e1000e_enable_receives(adapter);
goto link_up;
--
1.6.6.1
^ permalink raw reply related
* Re: [RFC Patch] net: reserve ports for applications using fixed port numbers
From: Bart Van Assche @ 2010-02-05 7:11 UTC (permalink / raw)
To: Amerigo Wang
Cc: linux-kernel, Eric Dumazet, linux-rdma, netdev, Neil Horman,
linux-sctp, David Miller
In-Reply-To: <20100203043332.3817.27932.sendpatchset@localhost.localdomain>
On Wed, Feb 3, 2010 at 5:30 AM, Amerigo Wang <amwang@redhat.com> wrote:
>
> This patch introduces /proc/sys/net/ipv4/ip_local_reserved_ports,
> it can be used like ip_local_port_range, but this is used to
> reserve ports for third-party applications which use fixed
> port numbers within ip_local_port_range.
>
> This only affects the applications which call socket functions
> like bind(2) with port number 0, to prevent the kernel getting the ports
> within the specified range for them. For applications which use fixed
> port number, it will have no effects.
>
> Any comments are welcome.
Relying on fixed port numbers is generally considered as a shortcoming
in the application. It would be helpful if you could explain more in
detail why port number reservation is necessary. Maybe there exists
another solution that does not require modifying the bind() system
call.
A quote from the UNIX socket FAQ (http://www.faqs.org/faqs/unix-faq/socket/):
4.10. How should I choose a port number for my server?
The list of registered port assignments can be found in STD 2 or RFC
1700. Choose one that isn't already registered, and isn't in
/etc/services on your system. It is also a good idea to let users
customize the port number in case of conflicts with other un-
registered port numbers in other servers. The best way of doing this
is hardcoding a service name, and using getservbyname() to lookup the
actual port number. This method allows users to change the port your
server binds to by simply editing the /etc/services file.
Bart.
^ permalink raw reply
* Re: [RFC Patch] net: reserve ports for applications using fixed port numbers
From: Cong Wang @ 2010-02-05 7:25 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-kernel, Eric Dumazet, linux-rdma, netdev, Neil Horman,
linux-sctp, David Miller
In-Reply-To: <e2e108261002042311w59ba1372ld9f8fb2f369ca434@mail.gmail.com>
Bart Van Assche wrote:
> On Wed, Feb 3, 2010 at 5:30 AM, Amerigo Wang <amwang@redhat.com> wrote:
>> This patch introduces /proc/sys/net/ipv4/ip_local_reserved_ports,
>> it can be used like ip_local_port_range, but this is used to
>> reserve ports for third-party applications which use fixed
>> port numbers within ip_local_port_range.
>>
>> This only affects the applications which call socket functions
>> like bind(2) with port number 0, to prevent the kernel getting the ports
>> within the specified range for them. For applications which use fixed
>> port number, it will have no effects.
>>
>> Any comments are welcome.
>
> Relying on fixed port numbers is generally considered as a shortcoming
> in the application. It would be helpful if you could explain more in
> detail why port number reservation is necessary. Maybe there exists
> another solution that does not require modifying the bind() system
> call.
>
The problem is that there are some existing applications which use
fixed port number, we don't have chances to change this for them,
thus making them working is desired, so they want to reserve these
port for those applications.
For example, if I have an appliction which uses port 40000, but
before this application starts, another application gets this port
number by bind() with port 0 (i.e. chosen by kernel), in this case,
that application will fail to start. Again, we don't have any chance
to change the source code of that application.
Hope this can make the problem clear.
Thanks.
^ permalink raw reply
* Re: linux-next: manual merge of the trivial tree with the net tree
From: Jiri Kosina @ 2010-02-05 8:39 UTC (permalink / raw)
To: David Miller
Cc: sfr, linux-next, linux-kernel, adam.buchbinder, netdev,
bhutchings
In-Reply-To: <20100204.202645.18974249.davem@davemloft.net>
On Thu, 4 Feb 2010, David Miller wrote:
> > Today's linux-next merge of the trivial tree got a conflict in
> > drivers/net/sfc/mcdi_pcol.h between commit
> > 5297a98d5dd6de86fe1e2ffc9ea60cdf59b71443 ("sfc: Update MCDI protocol
> > definitions") from the net tree and commit
> > 4887b438e6880c73c4b44d868211e70c1f3deaec ("Fix misspelling of
> > "successful" and variants in comments") from the trivial tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary.
>
> Ugh, this is the second spelling fix that's hit a conflict
> in the same exact tree.
>
> Please, submit these things to the subsystem maintainers instead
> of keeping them together in a totally seperate tree. That way
> we won't have to keep fighting these things.
Well, no problem with that.
Some maintainers just don't want to be buggered with such patches though,
and I always take care of sending this queue to Linus only when all the
trees which had conflict in linux-next are already in (and I do the
conflict resolution myself), so this should be exactly zero additional
work for subsystem maintainers.
But if you don't like this, I'll just start refusing all the trivial
patches touching net/ and drivers/net/ and will redirect them your way.
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply
* Re: [RFC Patch] net: reserve ports for applications using fixed portnumbers
From: Tetsuo Handa @ 2010-02-05 9:08 UTC (permalink / raw)
To: bvanassche
Cc: amwang, linux-kernel, eric.dumazet, linux-rdma, netdev, nhorman,
linux-sctp, davem
In-Reply-To: <4B6BC7E4.5070909@redhat.com>
Cong Wang wrote:
> The problem is that there are some existing applications which use
> fixed port number, we don't have chances to change this for them,
> thus making them working is desired, so they want to reserve these
> port for those applications.
>
> For example, if I have an appliction which uses port 40000, but
> before this application starts, another application gets this port
> number by bind() with port 0 (i.e. chosen by kernel), in this case,
> that application will fail to start. Again, we don't have any chance
> to change the source code of that application.
>
And there is a utility called "portreserved" (port reserve daemon).
http://fedoraproject.org/wiki/Features/Portreserve
But that utility cannot close the race window between "portreserved stops
reserving local port numbers" and "applications starts using local port
numbers which portreserved was reserving".
Thus, I think people want to have port reservation mechanism inside kernel
(if it has little impact).
^ permalink raw reply
* Re: [RFC 0/4] bond hashing revised
From: Jasper Spaans @ 2010-02-05 9:40 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David Miller, Jay Vosburgh, netdev@vger.kernel.org,
bonding-devel@lists.sourceforge.net
In-Reply-To: <20100204171118.917737392@vyatta.com>
On 04/02/10 17:11, Stephen Hemminger wrote:
> These have not been tested yet. I need to try them with some
> different flows/hardware to validate.
>
Glancing over these patches, all four of them look fine to me.
The only thing missing is some documentation in
Documentation/networking/bonding.txt (I have no idea what multiqueue is
supposed to do, but if you tell me I'll volunteer).
Cheers,
Jasper
--
Ir. Jasper Spaans
Fox-IT Experts in IT Security!
T: +31 (0) 15 284 79 99
KvK Haaglanden 27301624
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 10:00 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: davem, jonathan, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <20100204194744.GA4185@x200>
Alexey Dobriyan wrote:
> On Thu, Feb 04, 2010 at 06:04:34PM +0100, Patrick McHardy wrote:
>> Patrick McHardy wrote:
>>> Alexey Dobriyan wrote:
>>>> Jon Masters correctly points out that conntrack hash sizes
>>>> (nf_conntrack_htable_size) are global (not per-netns) and
>>>> modifiable at runtime via /sys/module/nf_conntrack/hashsize .
>>>>
>>>> Steps to reproduce:
>>>> clone(CLONE_NEWNET)
>>>> [grow /sys/module/nf_conntrack/hashsize]
>>>> exit()
>>>>
>>>> At netns exit we are going to scan random memory for conntracks to be killed.
>>>>
>>>> Apparently there is a code which deals with hashtable resize for
>>>> init_net (and it was there befode netns conntrack code), so prohibit
>>>> hashsize modification if there is more than one netns exists.
>>>>
>>>> To change hashtable sizes, you need to reload module.
>>>>
>>>> Expectation hashtable size was simply glued to a variable with no code
>>>> to rehash expectations, so it was a bug to allow writing to it.
>>>> Make "expect_hashsize" readonly.
>>>>
>>>> This is temporarily until we figure out what to do.
>>> How about alternatively moving nf_conntrack_hsize into the
>>> per-namespace struct? It doesn't look more complicated or
>>> intrusive and would allow to still change the init_net
>>> hashsize. Also seems less hackish :)
>> How about this (so far untested) patch? The htable_size is moved into
>> the per-namespace struct and initialized from the current (global)
>> value of nf_conntrack_htable_size. Changes through sysfs are still
>> permitted, but only affect the init namespace and newly created ones.
>
> No matter what we do, it's a hack!
>
>> Additionally I removed reinitializing the hash random value when
>> changing the hash size since that also requires to rehash in all
>> namespaces.
>
> I'm not fond of this, because we're not even closely going to allow changing
> hashtable size per-netns. As such having actual per-netns hashtable size
> just slows down everything.
Actually it doesn't seem like much more work to allow changing
table size, the main problem is that sysfs module parameters
don't seem to fit into the network namespace model at all.
Please be more specific about your suspected slowdowns.
What's "everything"? What's different about the hashsize
compared to the many members we already moved to per-netns
structs?
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 10:00 UTC (permalink / raw)
To: Jon Masters; +Cc: Alexey Dobriyan, davem, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <1265314734.2861.521.camel@tonnant>
Jon Masters wrote:
> On Thu, 2010-02-04 at 17:27 +0100, Patrick McHardy wrote:
>> Patrick McHardy wrote:
>>> Alexey Dobriyan wrote:
>>>> Jon Masters correctly points out that conntrack hash sizes
>>>> (nf_conntrack_htable_size) are global (not per-netns) and
>>>> modifiable at runtime via /sys/module/nf_conntrack/hashsize .
>>>>
>>>> Steps to reproduce:
>>>> clone(CLONE_NEWNET)
>>>> [grow /sys/module/nf_conntrack/hashsize]
>>>> exit()
>>>>
>>>> At netns exit we are going to scan random memory for conntracks to be killed.
>>>>
>>>> Apparently there is a code which deals with hashtable resize for
>>>> init_net (and it was there befode netns conntrack code), so prohibit
>>>> hashsize modification if there is more than one netns exists.
>>>>
>>>> To change hashtable sizes, you need to reload module.
>>>>
>>>> Expectation hashtable size was simply glued to a variable with no code
>>>> to rehash expectations, so it was a bug to allow writing to it.
>>>> Make "expect_hashsize" readonly.
>>>>
>>>> This is temporarily until we figure out what to do.
>>> How about alternatively moving nf_conntrack_hsize into the
>>> per-namespace struct? It doesn't look more complicated or
>>> intrusive and would allow to still change the init_net
>>> hashsize. Also seems less hackish :)
>> Just to avoid duplicate work, I'm currently trying that.
>
> Bah. I already worked a set of patches to do that as I mentioned, but
> you've probably done it by now - can clean up and post if not :)
Sorry, I missed that in your mail. I'm pretty much done, will finish
testing shortly.
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 10:03 UTC (permalink / raw)
To: Jon Masters; +Cc: Alexey Dobriyan, davem, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <1265314835.2861.524.camel@tonnant>
Jon Masters wrote:
> On Thu, 2010-02-04 at 18:04 +0100, Patrick McHardy wrote:
>>> How about alternatively moving nf_conntrack_hsize into the
>>> per-namespace struct? It doesn't look more complicated or
>>> intrusive and would allow to still change the init_net
>>> hashsize. Also seems less hackish :)
>> How about this (so far untested) patch? The htable_size is moved into
>> the per-namespace struct and initialized from the current (global)
>> value of nf_conntrack_htable_size. Changes through sysfs are still
>> permitted, but only affect the init namespace and newly created ones.
>
> I moved the random seed into the per-ns context aswell. I think that's
> better than having a global one, and you don't need to rehash all.
That's another possibility. But we don't loose anything by not
reseeding during resize. It also shouldn't be possible to determine
the seed from userspace in a namespace, so there's no real need
to use seperate values.
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Jon Masters @ 2010-02-05 10:11 UTC (permalink / raw)
To: Patrick McHardy
Cc: Alexey Dobriyan, davem, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <4B6BEC23.8020101@trash.net>
On Fri, 2010-02-05 at 11:00 +0100, Patrick McHardy wrote:
> Alexey Dobriyan wrote:
> > On Thu, Feb 04, 2010 at 06:04:34PM +0100, Patrick McHardy wrote:
> >> Patrick McHardy wrote:
> >>> Alexey Dobriyan wrote:
> >>>> Jon Masters correctly points out that conntrack hash sizes
> >>>> (nf_conntrack_htable_size) are global (not per-netns) and
> >>>> modifiable at runtime via /sys/module/nf_conntrack/hashsize .
> >>>>
> >>>> Steps to reproduce:
> >>>> clone(CLONE_NEWNET)
> >>>> [grow /sys/module/nf_conntrack/hashsize]
> >>>> exit()
> >>>>
> >>>> At netns exit we are going to scan random memory for conntracks to be killed.
> >>>>
> >>>> Apparently there is a code which deals with hashtable resize for
> >>>> init_net (and it was there befode netns conntrack code), so prohibit
> >>>> hashsize modification if there is more than one netns exists.
> >>>>
> >>>> To change hashtable sizes, you need to reload module.
> >>>>
> >>>> Expectation hashtable size was simply glued to a variable with no code
> >>>> to rehash expectations, so it was a bug to allow writing to it.
> >>>> Make "expect_hashsize" readonly.
> >>>>
> >>>> This is temporarily until we figure out what to do.
> >>> How about alternatively moving nf_conntrack_hsize into the
> >>> per-namespace struct? It doesn't look more complicated or
> >>> intrusive and would allow to still change the init_net
> >>> hashsize. Also seems less hackish :)
> >> How about this (so far untested) patch? The htable_size is moved into
> >> the per-namespace struct and initialized from the current (global)
> >> value of nf_conntrack_htable_size. Changes through sysfs are still
> >> permitted, but only affect the init namespace and newly created ones.
> >
> > No matter what we do, it's a hack!
> >
> >> Additionally I removed reinitializing the hash random value when
> >> changing the hash size since that also requires to rehash in all
> >> namespaces.
> >
> > I'm not fond of this, because we're not even closely going to allow changing
> > hashtable size per-netns. As such having actual per-netns hashtable size
> > just slows down everything.
>
> Actually it doesn't seem like much more work to allow changing
> table size, the main problem is that sysfs module parameters
> don't seem to fit into the network namespace model at all.
That was the reason I initially suggested we need a better way to expose
netns topology through sysfs, which I still think is a good idea. How
about this...it's dangerous as it is right now to leave things global. I
suggest leaving the existing sysfs module parameter that only actually
touches the init_net ct and get the rest fixed up, then adding support
for exposing the topology better in sysfs and tweaking per-ns bits.
But maybe you want to fix it all at the same time.
Jon.
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Jon Masters @ 2010-02-05 10:12 UTC (permalink / raw)
To: Patrick McHardy
Cc: Alexey Dobriyan, davem, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <4B6BECF8.1010508@trash.net>
On Fri, 2010-02-05 at 11:03 +0100, Patrick McHardy wrote:
> Jon Masters wrote:
> > On Thu, 2010-02-04 at 18:04 +0100, Patrick McHardy wrote:
> >>> How about alternatively moving nf_conntrack_hsize into the
> >>> per-namespace struct? It doesn't look more complicated or
> >>> intrusive and would allow to still change the init_net
> >>> hashsize. Also seems less hackish :)
> >> How about this (so far untested) patch? The htable_size is moved into
> >> the per-namespace struct and initialized from the current (global)
> >> value of nf_conntrack_htable_size. Changes through sysfs are still
> >> permitted, but only affect the init namespace and newly created ones.
> >
> > I moved the random seed into the per-ns context aswell. I think that's
> > better than having a global one, and you don't need to rehash all.
>
> That's another possibility. But we don't loose anything by not
> reseeding during resize. It also shouldn't be possible to determine
> the seed from userspace in a namespace, so there's no real need
> to use seperate values.
Right, the risk there is hypothetical at best. But there's little lost
in putting it in per-ns and then you can rehash and truly make them
independent, which I think is really what netns is all about.
Jon.
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Jon Masters @ 2010-02-05 10:14 UTC (permalink / raw)
To: Patrick McHardy
Cc: Alexey Dobriyan, davem, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <4B6BEC44.5000101@trash.net>
On Fri, 2010-02-05 at 11:00 +0100, Patrick McHardy wrote:
> Jon Masters wrote:
> > On Thu, 2010-02-04 at 17:27 +0100, Patrick McHardy wrote:
> >> Patrick McHardy wrote:
> >>> Alexey Dobriyan wrote:
> >>>> Jon Masters correctly points out that conntrack hash sizes
> >>>> (nf_conntrack_htable_size) are global (not per-netns) and
> >>>> modifiable at runtime via /sys/module/nf_conntrack/hashsize .
> >>>>
> >>>> Steps to reproduce:
> >>>> clone(CLONE_NEWNET)
> >>>> [grow /sys/module/nf_conntrack/hashsize]
> >>>> exit()
> >>>>
> >>>> At netns exit we are going to scan random memory for conntracks to be killed.
> >>>>
> >>>> Apparently there is a code which deals with hashtable resize for
> >>>> init_net (and it was there befode netns conntrack code), so prohibit
> >>>> hashsize modification if there is more than one netns exists.
> >>>>
> >>>> To change hashtable sizes, you need to reload module.
> >>>>
> >>>> Expectation hashtable size was simply glued to a variable with no code
> >>>> to rehash expectations, so it was a bug to allow writing to it.
> >>>> Make "expect_hashsize" readonly.
> >>>>
> >>>> This is temporarily until we figure out what to do.
> >>> How about alternatively moving nf_conntrack_hsize into the
> >>> per-namespace struct? It doesn't look more complicated or
> >>> intrusive and would allow to still change the init_net
> >>> hashsize. Also seems less hackish :)
> >> Just to avoid duplicate work, I'm currently trying that.
> >
> > Bah. I already worked a set of patches to do that as I mentioned, but
> > you've probably done it by now - can clean up and post if not :)
>
> Sorry, I missed that in your mail. I'm pretty much done, will finish
> testing shortly.
Oh, it's cool. I hacked it together on my test box but I'm happy to go
with whatever you post later, I will just try to be forthcoming next
time with my bits first to save you hassle. Please do keep CCing me on
these things and I'll try to test over the weekend as time permits.
Jon.
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 10:19 UTC (permalink / raw)
To: Jon Masters; +Cc: Alexey Dobriyan, davem, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <1265364693.2861.756.camel@tonnant>
Jon Masters wrote:
> On Fri, 2010-02-05 at 11:00 +0100, Patrick McHardy wrote:
>>>> How about this (so far untested) patch? The htable_size is moved into
>>>> the per-namespace struct and initialized from the current (global)
>>>> value of nf_conntrack_htable_size. Changes through sysfs are still
>>>> permitted, but only affect the init namespace and newly created ones.
>>> No matter what we do, it's a hack!
>>>
>>>> Additionally I removed reinitializing the hash random value when
>>>> changing the hash size since that also requires to rehash in all
>>>> namespaces.
>>> I'm not fond of this, because we're not even closely going to allow changing
>>> hashtable size per-netns. As such having actual per-netns hashtable size
>>> just slows down everything.
>> Actually it doesn't seem like much more work to allow changing
>> table size, the main problem is that sysfs module parameters
>> don't seem to fit into the network namespace model at all.
>
> That was the reason I initially suggested we need a better way to expose
> netns topology through sysfs, which I still think is a good idea. How
> about this...it's dangerous as it is right now to leave things global. I
> suggest leaving the existing sysfs module parameter that only actually
> touches the init_net ct and get the rest fixed up, then adding support
> for exposing the topology better in sysfs and tweaking per-ns bits.
>
> But maybe you want to fix it all at the same time.
No, right now I only want to fix the remaining bugs. I'll leave
everything else to the netns people.
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 10:21 UTC (permalink / raw)
To: Jon Masters; +Cc: Alexey Dobriyan, davem, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <1265364761.2861.757.camel@tonnant>
Jon Masters wrote:
> On Fri, 2010-02-05 at 11:03 +0100, Patrick McHardy wrote:
>> Jon Masters wrote:
>>> On Thu, 2010-02-04 at 18:04 +0100, Patrick McHardy wrote:
>>>>> How about alternatively moving nf_conntrack_hsize into the
>>>>> per-namespace struct? It doesn't look more complicated or
>>>>> intrusive and would allow to still change the init_net
>>>>> hashsize. Also seems less hackish :)
>>>> How about this (so far untested) patch? The htable_size is moved into
>>>> the per-namespace struct and initialized from the current (global)
>>>> value of nf_conntrack_htable_size. Changes through sysfs are still
>>>> permitted, but only affect the init namespace and newly created ones.
>>> I moved the random seed into the per-ns context aswell. I think that's
>>> better than having a global one, and you don't need to rehash all.
>> That's another possibility. But we don't loose anything by not
>> reseeding during resize. It also shouldn't be possible to determine
>> the seed from userspace in a namespace, so there's no real need
>> to use seperate values.
>
> Right, the risk there is hypothetical at best. But there's little lost
> in putting it in per-ns and then you can rehash and truly make them
> independent, which I think is really what netns is all about.
I don't disagree, but currently I'm trying to go for a minimal
version thats suitable for 2.6.33.
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 10:21 UTC (permalink / raw)
To: Jon Masters; +Cc: Alexey Dobriyan, davem, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <1265364850.2861.759.camel@tonnant>
Jon Masters wrote:
> On Fri, 2010-02-05 at 11:00 +0100, Patrick McHardy wrote:
>>>> Just to avoid duplicate work, I'm currently trying that.
>>> Bah. I already worked a set of patches to do that as I mentioned, but
>>> you've probably done it by now - can clean up and post if not :)
>> Sorry, I missed that in your mail. I'm pretty much done, will finish
>> testing shortly.
>
> Oh, it's cool. I hacked it together on my test box but I'm happy to go
> with whatever you post later, I will just try to be forthcoming next
> time with my bits first to save you hassle. Please do keep CCing me on
> these things and I'll try to test over the weekend as time permits.
Will do, thanks for all your help Jon.
^ permalink raw reply
* Re: netfilter/iptables and network interface names
From: Patrick McHardy @ 2010-02-05 10:27 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: Thomas Woerner, netdev, Netfilter Developer Mailing List
In-Reply-To: <alpine.LSU.2.01.1002041944120.30159@obet.zrqbmnf.qr>
Jan Engelhardt wrote:
> On Thursday 2010-02-04 17:49, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>>> The kernel also forbids ".".
>>> My first thought was the same, but:
>>>
>>> a) Interestingly, it does not prohibit '.'
>>>
>>> ip tunnel add foo0.3 mode sit local 1.2.3.4 remote 5.6.7.8
>>>
>>> b) The '.' is to be seen as valid as far as xtables.c goes, so
>>> as to match VLAN interfaces.
>> Ah of course, its only invalid at the beginning of the name.
>>
> Nope:
>
> 19:44 borg:/home/jengelh # ip tunnel add .3 mode sit local 1.2.3.6 remote
> 5.6.7.7
> 19:44 borg:/home/jengelh # ip a
> 17: .3: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN
> link/sit 1.2.3.6 peer 5.6.7.7
OK you're right, but this seems to be an oversight. The names
are checked during rename, but not on device registration.
^ permalink raw reply
* Re: netfilter/iptables and network interface names
From: Patrick McHardy @ 2010-02-05 10:32 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: Thomas Woerner, netdev, Netfilter Developer Mailing List
In-Reply-To: <alpine.LSU.2.01.1002041542150.13435@obet.zrqbmnf.qr>
Jan Engelhardt wrote:
> On Thursday 2010-02-04 15:31, Patrick McHardy wrote:
>> I don't think there is a reason for this limitation in iptables,
>> so why not simply remove it?
>
> Like this?
>
> parent 350661a6eb089f3e54e67e022db9e16ea280499f (v1.4.6-7-g350661a)
> commit 02020c8fb965bbedae9eb43d9a9b964c46388cc3
> Author: Jan Engelhardt <jengelh@medozas.de>
> Date: Thu Feb 4 15:46:03 2010 +0100
>
> Lift restrictions on interface names
>
> The kernel has few restrictions.
>
> Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
OK this looks fine. But I guess we still need to properly escape
names in iptables-save as Thomas has pointed out.
> ---
> xtables.c | 11 +++++------
> 1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/xtables.c b/xtables.c
> index 63c5db7..f3baf84 100644
> --- a/xtables.c
> +++ b/xtables.c
> @@ -450,7 +450,7 @@ u_int16_t xtables_parse_port(const char *port, const char *proto)
> void xtables_parse_interface(const char *arg, char *vianame,
> unsigned char *mask)
> {
> - int vialen = strlen(arg);
> + unsigned int vialen = strlen(arg);
> unsigned int i;
>
> memset(mask, 0, IFNAMSIZ);
> @@ -462,7 +462,7 @@ void xtables_parse_interface(const char *arg, char *vianame,
> " (%i)", arg, IFNAMSIZ-1);
>
> strcpy(vianame, arg);
> - if ((vialen == 0) || (vialen == 1 && vianame[0] == '+'))
> + if (vialen == 0)
> memset(mask, 0, IFNAMSIZ);
> else if (vianame[vialen - 1] == '+') {
> memset(mask, 0xFF, vialen - 1);
> @@ -473,12 +473,11 @@ void xtables_parse_interface(const char *arg, char *vianame,
> memset(mask, 0xFF, vialen + 1);
> memset(mask + vialen + 1, 0, IFNAMSIZ - vialen - 1);
> for (i = 0; vianame[i]; i++) {
> - if (vianame[i] == ':' ||
> - vianame[i] == '!' ||
> - vianame[i] == '*') {
> + if (vianame[i] == '/' ||
> + vianame[i] == ' ') {
> fprintf(stderr,
> "Warning: weird character in interface"
> - " `%s' (No aliases, :, ! or *).\n",
> + " `%s' ('/' and ' ' are not allowed by the kernel).\n",
> vianame);
> break;
> }
^ permalink raw reply
* Re: [RFC 3/4] bond: support more Layer 4 protocols
From: Patrick McHardy @ 2010-02-05 10:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, Jay Vosburgh, netdev, bonding-devel
In-Reply-To: <20100204171241.393163298@vyatta.com>
Stephen Hemminger wrote:
> +/* Map of protocols with standard ports available to include in hash */
> +static const bool has_layer4[256] = {
> + [IPPROTO_TCP] = 1,
> + [IPPROTO_UDP] = 1,
> + [IPPROTO_UDPLITE] = 1,
> + [IPPROTO_SCTP] = 1,
> + [IPPROTO_DCCP] = 1,
> + [IPPROTO_ESP] = 1,
> +};
> +
How about using a bitmap or u8s to keep this more compact?
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 11:16 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: davem, jonathan, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <4B6BEC23.8020101@trash.net>
Patrick McHardy wrote:
> Alexey Dobriyan wrote:
>>> How about this (so far untested) patch? The htable_size is moved into
>>> the per-namespace struct and initialized from the current (global)
>>> value of nf_conntrack_htable_size. Changes through sysfs are still
>>> permitted, but only affect the init namespace and newly created ones.
>> No matter what we do, it's a hack!
>>
>>> Additionally I removed reinitializing the hash random value when
>>> changing the hash size since that also requires to rehash in all
>>> namespaces.
>> I'm not fond of this, because we're not even closely going to allow changing
>> hashtable size per-netns. As such having actual per-netns hashtable size
>> just slows down everything.
>
> Actually it doesn't seem like much more work to allow changing
> table size, the main problem is that sysfs module parameters
> don't seem to fit into the network namespace model at all.
>
> Please be more specific about your suspected slowdowns.
> What's "everything"? What's different about the hashsize
> compared to the many members we already moved to per-netns
> structs?
OK testing looks fine, although I'm quite surprised that its actually
possible to change module parameters from within non-init namespaces.
How is this supposed to work at all? I don't see how sysfs could
possibly provide a network namespace context ...
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Alexey Dobriyan @ 2010-02-05 11:19 UTC (permalink / raw)
To: Patrick McHardy; +Cc: davem, jonathan, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <4B6BFE29.1040204@trash.net>
On Fri, Feb 5, 2010 at 1:16 PM, Patrick McHardy <kaber@trash.net> wrote:
> OK testing looks fine, although I'm quite surprised that its actually
> possible to change module parameters from within non-init namespaces.
> How is this supposed to work at all? I don't see how sysfs could
> possibly provide a network namespace context ...
You can do in write hook
if (!net_eq(current->nsproxy->net_ns, &init_net))
return -EINVAL;
^ permalink raw reply
* Re: [RFC Patch v2] net: reserve ports for applications using fixedport numbers
From: Tetsuo Handa @ 2010-02-05 11:21 UTC (permalink / raw)
To: amwang
Cc: linux-kernel, linux-security-module, opurdila, eric.dumazet,
linux-rdma, netdev, nhorman, linux-sctp, davem
In-Reply-To: <4B6BA16E.3010002@redhat.com>
Cong Wang wrote:
> Oh, IIUC, TOMOYO is something like SELinux?
Yes. It is a policy based mandatory access control implementation which is
applied to not only non root users but also root user. If MAC is enabled,
root user cannot freely modify via sysctl() or /proc/sys interface.
> So, it is somewhat weird to let users to use TOMOYO to reserve
> the ports with MAC.
To add reserved port
echo deny_autobind 0-1023 | ccs-loadpolicy -e
echo deny_autobind 3128 | ccs-loadpolicy -e
echo deny_autobind 8080 | ccs-loadpolicy -e
and to delete reserved port
echo delete deny_autobind 0-1023 | ccs-loadpolicy -e
echo delete deny_autobind 3128 | ccs-loadpolicy -e
echo delete deny_autobind 8080 | ccs-loadpolicy -e
That's all. Quite easy.
> For normal users /proc interface seems more friendly.
I think /proc/sys/net/ipv4/ip_local_reserved_ports interface wants
"struct list_head" for handling multiple sets of min/max pairs. I'm using
http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/security/ccsecurity/autobind.c#L29
for that purpose.
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 11:22 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: davem, jonathan, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <b6fcc0a1002050319p2ba0ff30y4bc8bdba08ff65d9@mail.gmail.com>
Alexey Dobriyan wrote:
> On Fri, Feb 5, 2010 at 1:16 PM, Patrick McHardy <kaber@trash.net> wrote:
>> OK testing looks fine, although I'm quite surprised that its actually
>> possible to change module parameters from within non-init namespaces.
>> How is this supposed to work at all? I don't see how sysfs could
>> possibly provide a network namespace context ...
>
>
> You can do in write hook
>
> if (!net_eq(current->nsproxy->net_ns, &init_net))
> return -EINVAL;
Right, I see. So we could actually make resizing work for all
namespaces quite easily. Is there any reason not to do this?
^ permalink raw reply
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Alexey Dobriyan @ 2010-02-05 11:23 UTC (permalink / raw)
To: Patrick McHardy; +Cc: davem, jonathan, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <b6fcc0a1002050319p2ba0ff30y4bc8bdba08ff65d9@mail.gmail.com>
On Fri, Feb 5, 2010 at 1:19 PM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> On Fri, Feb 5, 2010 at 1:16 PM, Patrick McHardy <kaber@trash.net> wrote:
>> OK testing looks fine, although I'm quite surprised that its actually
>> possible to change module parameters from within non-init namespaces.
>> How is this supposed to work at all? I don't see how sysfs could
>> possibly provide a network namespace context ...
>
>
> You can do in write hook
>
> if (!net_eq(current->nsproxy->net_ns, &init_net))
> return -EINVAL;
-EPERM of course.
--
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: linux-next: manual merge of the trivial tree with the net tree
From: Jiri Kosina @ 2010-02-05 11:23 UTC (permalink / raw)
To: David Miller
Cc: sfr, linux-next, linux-kernel, adam.buchbinder, netdev,
bhutchings
In-Reply-To: <alpine.LNX.2.00.1002050935470.15395@pobox.suse.cz>
On Fri, 5 Feb 2010, Jiri Kosina wrote:
> > > Today's linux-next merge of the trivial tree got a conflict in
> > > drivers/net/sfc/mcdi_pcol.h between commit
> > > 5297a98d5dd6de86fe1e2ffc9ea60cdf59b71443 ("sfc: Update MCDI protocol
> > > definitions") from the net tree and commit
> > > 4887b438e6880c73c4b44d868211e70c1f3deaec ("Fix misspelling of
> > > "successful" and variants in comments") from the trivial tree.
> > >
> > > I fixed it up (see below) and can carry the fix as necessary.
> >
> > Ugh, this is the second spelling fix that's hit a conflict
> > in the same exact tree.
> >
> > Please, submit these things to the subsystem maintainers instead
> > of keeping them together in a totally seperate tree. That way
> > we won't have to keep fighting these things.
>
> Well, no problem with that.
>
> Some maintainers just don't want to be buggered with such patches though,
> and I always take care of sending this queue to Linus only when all the
> trees which had conflict in linux-next are already in (and I do the
> conflict resolution myself), so this should be exactly zero additional
> work for subsystem maintainers.
>
> But if you don't like this, I'll just start refusing all the trivial
> patches touching net/ and drivers/net/ and will redirect them your way.
Ayway, below is the hunk that I have already dropped from my tree (so that
conflict in linux-next is gone), please feel free to apply it to your
tree, and let me known whether you want me to reject all furutre patches
touching net/ and driver/net/ to be refused on my side and redirected your
way, or if you are fine with me handling the conflict resolution before I
push them to Linus.
Thanks.
From: Adam Buchbinder <adam.buchbinder@gmail.com>
Subject: NET: Fix misspelling of "successful" and variants in comments.
Some comments misspell "successful" or variants of the word; this
fixes them. No code changes.
Signed-off-by: Adam Buchbinder <adam.buchbinder@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
diff --git a/drivers/net/sfc/mcdi_pcol.h b/drivers/net/sfc/mcdi_pcol.h
index 2a85360..f61e1de 100644
--- a/drivers/net/sfc/mcdi_pcol.h
+++ b/drivers/net/sfc/mcdi_pcol.h
@@ -853,7 +853,7 @@
* Poll for BIST completion
*
* Returns a single status code, and a binary blob of phy-specific
- * bist output. If the driver can't succesfully parse the BIST output,
+ * bist output. If the driver can't successfully parse the BIST output,
* it should still respect the Pass/Fail in OUT.RESULT.
*
* Locks required: PHY_LOCK if doing a PHY BIST
diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
index 64cdfeb..40ee5f6 100644
--- a/drivers/net/wimax/i2400m/fw.c
+++ b/drivers/net/wimax/i2400m/fw.c
@@ -1595,7 +1595,7 @@ int i2400m_dev_bootstrap(struct i2400m *i2400m, enum i2400m_bri flags)
i2400m->fw_name = fw_name;
ret = i2400m_fw_bootstrap(i2400m, fw, flags);
release_firmware(fw);
- if (ret >= 0) /* firmware loaded succesfully */
+ if (ret >= 0) /* firmware loaded successfully */
break;
i2400m->fw_name = NULL;
}
diff --git a/drivers/net/wireless/rt2x00/rt2x00link.c b/drivers/net/wireless/rt2x00/rt2x00link.c
index 0efbf5a..ffee9f8 100644
--- a/drivers/net/wireless/rt2x00/rt2x00link.c
+++ b/drivers/net/wireless/rt2x00/rt2x00link.c
@@ -240,7 +240,7 @@ void rt2x00link_update_stats(struct rt2x00_dev *rt2x00dev,
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
/*
- * Frame was received successfully since non-succesfull
+ * Frame was received successfully since non-successful
* frames would have been dropped by the hardware.
*/
qual->rx_success++;
diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index 0a751e7..8b8c500 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -200,7 +200,7 @@ static void rt2x00usb_interrupt_txdone(struct urb *urb)
* Obtain the status about this packet.
* Note that when the status is 0 it does not mean the
* frame was send out correctly. It only means the frame
- * was succesfully pushed to the hardware, we have no
+ * was successfully pushed to the hardware, we have no
* way to determine the transmission status right now.
* (Only indirectly by looking at the failed TX counters
* in the register).
^ permalink raw reply related
* Re: [PATCH for 2.6.33] conntrack: restrict runtime hashsize modifications
From: Patrick McHardy @ 2010-02-05 11:25 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: davem, jonathan, eric.dumazet, netdev, netfilter-devel
In-Reply-To: <4B6BFF69.6050503@trash.net>
[-- Attachment #1: Type: text/plain, Size: 715 bytes --]
Patrick McHardy wrote:
> Alexey Dobriyan wrote:
>> On Fri, Feb 5, 2010 at 1:16 PM, Patrick McHardy <kaber@trash.net> wrote:
>>> OK testing looks fine, although I'm quite surprised that its actually
>>> possible to change module parameters from within non-init namespaces.
>>> How is this supposed to work at all? I don't see how sysfs could
>>> possibly provide a network namespace context ...
>>
>> You can do in write hook
>>
>> if (!net_eq(current->nsproxy->net_ns, &init_net))
>> return -EINVAL;
>
> Right, I see. So we could actually make resizing work for all
> namespaces quite easily. Is there any reason not to do this?
>
Something like this (untested) patch on top of the previous one.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 2117 bytes --]
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index ef1c856..212dac3 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -31,6 +31,7 @@
#include <linux/socket.h>
#include <linux/mm.h>
#include <linux/rculist_nulls.h>
+#include <linux/nsproxy.h>
#include <net/netfilter/nf_conntrack.h>
#include <net/netfilter/nf_conntrack_l3proto.h>
@@ -1194,6 +1195,7 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
unsigned int hashsize, old_size;
struct hlist_nulls_head *hash, *old_hash;
struct nf_conntrack_tuple_hash *h;
+ struct net *net = current->nsproxy->net_ns;
/* On boot, we can set this without any fancy locking. */
if (!nf_conntrack_htable_size)
@@ -1213,9 +1215,9 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
* though since that required taking the lock.
*/
spin_lock_bh(&nf_conntrack_lock);
- for (i = 0; i < init_net.ct.htable_size; i++) {
- while (!hlist_nulls_empty(&init_net.ct.hash[i])) {
- h = hlist_nulls_entry(init_net.ct.hash[i].first,
+ for (i = 0; i < net->ct.htable_size; i++) {
+ while (!hlist_nulls_empty(&net->ct.hash[i])) {
+ h = hlist_nulls_entry(net->ct.hash[i].first,
struct nf_conntrack_tuple_hash, hnnode);
hlist_nulls_del_rcu(&h->hnnode);
bucket = __hash_conntrack(&h->tuple, hashsize,
@@ -1223,13 +1225,13 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
hlist_nulls_add_head_rcu(&h->hnnode, &hash[bucket]);
}
}
- old_size = init_net.ct.htable_size;
- old_vmalloced = init_net.ct.hash_vmalloc;
- old_hash = init_net.ct.hash;
+ old_size = net->ct.htable_size;
+ old_vmalloced = net->ct.hash_vmalloc;
+ old_hash = net->ct.hash;
- init_net.ct.htable_size = nf_conntrack_htable_size = hashsize;
- init_net.ct.hash_vmalloc = vmalloced;
- init_net.ct.hash = hash;
+ net->ct.htable_size = nf_conntrack_htable_size = hashsize;
+ net->ct.hash_vmalloc = vmalloced;
+ net->ct.hash = hash;
spin_unlock_bh(&nf_conntrack_lock);
nf_ct_free_hashtable(old_hash, old_vmalloced, old_size);
^ permalink raw reply related
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