* Re: [slab poison overwritten] Re: [GIT] Networking
From: David Miller @ 2011-03-22 3:40 UTC (permalink / raw)
To: horms
Cc: eric.dumazet, mingo, torvalds, akpm, netdev, linux-kernel,
a.p.zijlstra, tglx, arnd, penberg, ja, hans
In-Reply-To: <20110322011801.GE27019@verge.net.au>
From: Simon Horman <horms@verge.net.au>
Date: Tue, 22 Mar 2011 10:18:01 +0900
> IPVS: Use global mutex in ip_vs_app.c
>
> As part of the work to make IPVS network namespace aware
> __ip_vs_app_mutex was replaced by a per-namespace lock,
> ipvs->app_mutex. ipvs->app_key is also supplied for debugging purposes.
>
> Unfortunately this implementation results in ipvs->app_key residing
> in non-static storage which at the very least causes a lockdep warning.
>
> This patch takes the rather heavy-handed approach of reinstating
> __ip_vs_app_mutex which will cover access to the ipvs->list_head
> of all network namespaces.
...
> Signed-off-by: Simon Horman <horms@verge.net.au>
Applied, thanks Simon.
^ permalink raw reply
* Re: [slab poison overwritten] Re: [GIT] Networking
From: David Miller @ 2011-03-22 3:39 UTC (permalink / raw)
To: horms
Cc: eric.dumazet, mingo, torvalds, akpm, netdev, linux-kernel,
a.p.zijlstra, tglx, arnd, penberg, ja
In-Reply-To: <20110321221357.GF22625@verge.net.au>
From: Simon Horman <horms@verge.net.au>
Date: Tue, 22 Mar 2011 07:13:58 +0900
> On Mon, Mar 21, 2011 at 09:15:40PM +0100, Eric Dumazet wrote:
>> [PATCH] ipvs: fix a typo in __ip_vs_control_init()
>>
>> Reported-by: Ingo Molnar <mingo@elte.hu>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> Cc: Simon Horman <horms@verge.net.au>
>> Cc: Julian Anastasov <ja@ssi.bg>
>
> Acked-by: Simon Horman <horms@verge.net.au>
Applied.
^ permalink raw reply
* Re: [PATCH 2/4] drivers/net/fec: Use constants instead of magic numbers
From: Shawn Guo @ 2011-03-22 2:50 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
In-Reply-To: <264d7eca14554038e57399850ddd7fa4491a7484.1300724245.git.LW@KARO-electronics.de>
On Mon, Mar 21, 2011 at 05:37:34PM +0100, Lothar Waßmann wrote:
> - No functional change.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
Acked-by: Shawn Guo <shawn.guo@freescale.com>
--
Regards,
Shawn
^ permalink raw reply
* Re: [PATCH 1/4] drivers/net/fec: Cleanup
From: Shawn Guo @ 2011-03-22 2:49 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, u.kleine-koenig
In-Reply-To: <cae922ff2a4ba144baf3d50f5d9fddd13d50e68b.1300724245.git.LW@KARO-electronics.de>
On Mon, Mar 21, 2011 at 05:37:33PM +0100, Lothar Waßmann wrote:
> - Whitespace cleanup
> - Convert indentation spaces to TABs
> - Spelling fixes
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
>
Acked-by: Shawn Guo <shawn.guo@freescale.com>
--
Regards,
Shawn
^ permalink raw reply
* Re: [PATCH][v3] dev : fix mtu check when TSO is enabled
From: Eric W. Biederman @ 2011-03-22 3:02 UTC (permalink / raw)
To: Jesse Gross
Cc: Stephen Hemminger, Daniel Lezcano, David Miller, eric.dumazet,
kaber, nightnord, netdev
In-Reply-To: <AANLkTikFsXmLV-CXD-AjthUhiJ834-G6_F9LrRNaZYTR@mail.gmail.com>
Jesse Gross <jesse@nicira.com> writes:
> On Mon, Mar 21, 2011 at 3:58 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Stephen Hemminger <shemminger@vyatta.com> writes:
>>
>>> On Wed, 16 Mar 2011 17:19:14 +0100
>>> Daniel Lezcano <daniel.lezcano@free.fr> wrote:
>>>
>>>> On 03/16/2011 04:35 PM, Stephen Hemminger wrote:
>>>> > On Wed, 16 Mar 2011 14:56:09 +0100
>>>> > Daniel Lezcano<daniel.lezcano@free.fr> wrote:
>>>> >
>>>> >> On 03/15/2011 07:17 PM, Stephen Hemminger wrote:
>>>> >>> On Tue, 15 Mar 2011 14:57:40 +0100
>>>> >>> Daniel Lezcano<daniel.lezcano@free.fr> wrote:
>>>> >>>
>>>> >>>> On 03/15/2011 12:59 AM, David Miller wrote:
>>>> >>>>> From: Daniel Lezcano<daniel.lezcano@free.fr>
>>>> >>>>> Date: Mon, 14 Mar 2011 21:39:50 +0100
>>>> >>>>>
>>>> >>>>>> + len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
>>>> >>>>>> + if (skb->len< len)
>>>> >>>>>> + return true;
>>>> >>>>> This is not a correct translation of the original test:
>>>> >>>>>
>>>> >>>>>> - (skb->len> (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
>>>> >>>>> You need to use "<=" in your version, which currently rejects all
>>>> >>>>> full sized frames. :-)
>>>> >>>> Right, thanks.
>>>> >>>>
>>>> >>>>>> +
>>>> >>>>>> + /* if TSO is enabled, we don't care about the length as the packet
>>>> >>>>>> + * could be forwarded without being segmented before
>>>> >>>>>> + */
>>>> >>>>>> + if (skb->dev&& skb->dev->features& NETIF_F_TSO)
>>>> >>>>>> + return true;
>>>> >>>>> I am trying to understand why you aren't simply checking also if this
>>>> >>>>> is a segmented frame? Perhaps skb_is_gso()&& device has NETIF_F_TSO
>>>> >>>>> set?
>>>> >>>> Maybe I am misunderstanding but the packet was forwarded by another device.
>>>> >>>> In our case from macvlan:
>>>> >>>>
>>>> >>>> macvlan_start_xmit
>>>> >>>> macvlan_queue_xmit
>>>> >>>> dest->forward
>>>> >>>> dev_skb_forward
>>>> >>>>
>>>> >>>> When we reached dev_skb_forward, that means we passed through
>>>> >>>> dev_hard_start_xmit where the packet was already segmented so we should
>>>> >>>> exit at the first test (skb->len< len). I don't see the point of adding
>>>> >>>> the skb_is_gso.
>>>> >>>> But maybe I am missing something, can you explain ?
>>>> >>> The macvlan device only has one downstream device (slave).
>>>> >>> If kernel is working properly, macvlan device should have a subset
>>>> >>> of the features of the underlying device
>>>> >> Right, dev->features = lowerdev->features& MACVLAN_FEATURES
>>>> >>
>>>> >>> and macvlan device should
>>>> >>> have same MTU as underlying device.
>>>> >> Right,
>>>> >>
>>>> >> ...
>>>> >>
>>>> >> if (!tb[IFLA_MTU])
>>>> >> dev->mtu = lowerdev->mtu;
>>>> >>
>>>> >> ...
>>>> >>> If the feature/MTU flags
>>>> >>> were correct, then the path calling macvlan should be respecting
>>>> >>> the MTU.
>>>> >> But if the TSO is enabled on the macvlan (inherited from eg e1000), the
>>>> >> packet won't be fragmented to the mtu size no ?
>>>> > That is the responsiblity of the hardware that receives the packet.
>>>> > Macvlan should be passing it through to the lowerdev and since the hardware
>>>> > supports TSO, it will fragment it.
>>>>
>>>> Ok, but in the case the macvlan is in bridge mode, the dev_skb_forward
>>>> function will forward the packet (which is not fragmented) to to another
>>>> macvlan port without going through the hardware driver. In this
>>>> function, the packet length is checked against the mtu size and of
>>>> course the packet is dropped in case the lower device support the TSO
>>>> (if the packet is larger than the mtu size). Dave suggested to check
>>>> skb_is_gso and against the TSO feature of the macvlan but I don't
>>>> understand why we should check skb_is_gso too.
>>>>
>>>> if (skb_is_gso(skb)&& (skb->dev&& skb->dev->features& NETIF_F_TSO))
>>>> return true;
>>>>
>>>>
>>>
>>> Then it is up to macvlan to do the same thing as bridge code.
>>>
>>> static inline unsigned packet_length(const struct sk_buff *skb)
>>> {
>>> return skb->len - (skb->protocol == htons(ETH_P_8021Q) ? VLAN_HLEN : 0);
>>> }
>>
>> Which is incorrect in this context. skb->len at this point in the code
>> includes the ethernet header, as we are down below dev_queue_xmit.
>>
>> The test really does need to be dev->mtu + dev->hard_header_len.
>>
>> As for conditionally include the VLAN_HLEN that is likely doable but I
>> think if we are being pedantic that case needs to deal with accelerated
>> vlan headers.
>
> If vlan acceleration is in use then the protocol is not ETH_P_8021Q
> and the tag is not included in skb->len.
Except for the case of double tagged packets.
Eric
^ permalink raw reply
* Re: [RFC] net: fix ethtool->set_flags not intended -EINVAL return value
From: Jesse Gross @ 2011-03-22 2:41 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Ben Hutchings, Amit Salecha, Eric Dumazet, Jesper Dangaard Brouer,
Alexander Duyck, netdev, Neil Horman
In-Reply-To: <20110321151015.GA2209@redhat.com>
On Mon, Mar 21, 2011 at 8:10 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> After commit d5dbda23804156ae6f35025ade5307a49d1db6d7 "ethtool: Add
> support for vlan accleration.", drivers that have NETIF_F_HW_VLAN_TX,
> and/or NETIF_F_HW_VLAN_RX feature, but do not allow enable/disable vlan
> acceleration via ethtool set_flags, always return -EINVAL from that
> function. Fix by returning -EINVAL only if requested features do
> not match current settings and can not be changed by driver.
Looks good to me, thanks for fixing this.
^ permalink raw reply
* Re: [PATCH][v3] dev : fix mtu check when TSO is enabled
From: Jesse Gross @ 2011-03-22 2:31 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Stephen Hemminger, Daniel Lezcano, David Miller, eric.dumazet,
kaber, nightnord, netdev
In-Reply-To: <m1pqpkf5cw.fsf@fess.ebiederm.org>
On Mon, Mar 21, 2011 at 3:58 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Stephen Hemminger <shemminger@vyatta.com> writes:
>
>> On Wed, 16 Mar 2011 17:19:14 +0100
>> Daniel Lezcano <daniel.lezcano@free.fr> wrote:
>>
>>> On 03/16/2011 04:35 PM, Stephen Hemminger wrote:
>>> > On Wed, 16 Mar 2011 14:56:09 +0100
>>> > Daniel Lezcano<daniel.lezcano@free.fr> wrote:
>>> >
>>> >> On 03/15/2011 07:17 PM, Stephen Hemminger wrote:
>>> >>> On Tue, 15 Mar 2011 14:57:40 +0100
>>> >>> Daniel Lezcano<daniel.lezcano@free.fr> wrote:
>>> >>>
>>> >>>> On 03/15/2011 12:59 AM, David Miller wrote:
>>> >>>>> From: Daniel Lezcano<daniel.lezcano@free.fr>
>>> >>>>> Date: Mon, 14 Mar 2011 21:39:50 +0100
>>> >>>>>
>>> >>>>>> + len = dev->mtu + dev->hard_header_len + VLAN_HLEN;
>>> >>>>>> + if (skb->len< len)
>>> >>>>>> + return true;
>>> >>>>> This is not a correct translation of the original test:
>>> >>>>>
>>> >>>>>> - (skb->len> (dev->mtu + dev->hard_header_len + VLAN_HLEN)))) {
>>> >>>>> You need to use "<=" in your version, which currently rejects all
>>> >>>>> full sized frames. :-)
>>> >>>> Right, thanks.
>>> >>>>
>>> >>>>>> +
>>> >>>>>> + /* if TSO is enabled, we don't care about the length as the packet
>>> >>>>>> + * could be forwarded without being segmented before
>>> >>>>>> + */
>>> >>>>>> + if (skb->dev&& skb->dev->features& NETIF_F_TSO)
>>> >>>>>> + return true;
>>> >>>>> I am trying to understand why you aren't simply checking also if this
>>> >>>>> is a segmented frame? Perhaps skb_is_gso()&& device has NETIF_F_TSO
>>> >>>>> set?
>>> >>>> Maybe I am misunderstanding but the packet was forwarded by another device.
>>> >>>> In our case from macvlan:
>>> >>>>
>>> >>>> macvlan_start_xmit
>>> >>>> macvlan_queue_xmit
>>> >>>> dest->forward
>>> >>>> dev_skb_forward
>>> >>>>
>>> >>>> When we reached dev_skb_forward, that means we passed through
>>> >>>> dev_hard_start_xmit where the packet was already segmented so we should
>>> >>>> exit at the first test (skb->len< len). I don't see the point of adding
>>> >>>> the skb_is_gso.
>>> >>>> But maybe I am missing something, can you explain ?
>>> >>> The macvlan device only has one downstream device (slave).
>>> >>> If kernel is working properly, macvlan device should have a subset
>>> >>> of the features of the underlying device
>>> >> Right, dev->features = lowerdev->features& MACVLAN_FEATURES
>>> >>
>>> >>> and macvlan device should
>>> >>> have same MTU as underlying device.
>>> >> Right,
>>> >>
>>> >> ...
>>> >>
>>> >> if (!tb[IFLA_MTU])
>>> >> dev->mtu = lowerdev->mtu;
>>> >>
>>> >> ...
>>> >>> If the feature/MTU flags
>>> >>> were correct, then the path calling macvlan should be respecting
>>> >>> the MTU.
>>> >> But if the TSO is enabled on the macvlan (inherited from eg e1000), the
>>> >> packet won't be fragmented to the mtu size no ?
>>> > That is the responsiblity of the hardware that receives the packet.
>>> > Macvlan should be passing it through to the lowerdev and since the hardware
>>> > supports TSO, it will fragment it.
>>>
>>> Ok, but in the case the macvlan is in bridge mode, the dev_skb_forward
>>> function will forward the packet (which is not fragmented) to to another
>>> macvlan port without going through the hardware driver. In this
>>> function, the packet length is checked against the mtu size and of
>>> course the packet is dropped in case the lower device support the TSO
>>> (if the packet is larger than the mtu size). Dave suggested to check
>>> skb_is_gso and against the TSO feature of the macvlan but I don't
>>> understand why we should check skb_is_gso too.
>>>
>>> if (skb_is_gso(skb)&& (skb->dev&& skb->dev->features& NETIF_F_TSO))
>>> return true;
>>>
>>>
>>
>> Then it is up to macvlan to do the same thing as bridge code.
>>
>> static inline unsigned packet_length(const struct sk_buff *skb)
>> {
>> return skb->len - (skb->protocol == htons(ETH_P_8021Q) ? VLAN_HLEN : 0);
>> }
>
> Which is incorrect in this context. skb->len at this point in the code
> includes the ethernet header, as we are down below dev_queue_xmit.
>
> The test really does need to be dev->mtu + dev->hard_header_len.
>
> As for conditionally include the VLAN_HLEN that is likely doable but I
> think if we are being pedantic that case needs to deal with accelerated
> vlan headers.
If vlan acceleration is in use then the protocol is not ETH_P_8021Q
and the tag is not included in skb->len.
^ permalink raw reply
* Re: [PATCH net-next-2.6 2/6] be2net: fix to not drop unfiltered vlan pkts
From: Jesse Gross @ 2011-03-22 2:21 UTC (permalink / raw)
To: Sathya.Perla; +Cc: netdev
In-Reply-To: <3367B80B08154D42A3B2BC708B5D41F63CC492C950@EXMAIL.ad.emulex.com>
On Sun, Mar 20, 2011 at 10:41 PM, <Sathya.Perla@emulex.com> wrote:
> Hi, can you pls clarify what you mean by the "new vlan model" below...
The interface that drivers use to pass vlan information to the
networking core has changed starting in 2.6.37, although the old
methods still exist for compatibility until all drivers have been
switched over. The idea is that drivers should no longer need any
knowledge of the vlan group and should instead simply pass along
whatever tagging information was received. With this patch you are
implicitly relying on the changes because you are giving a NULL vlan
group in some situations, which now works because the compatibility
functions ignore it.
An example of a driver conversion is in 7a8fc77b3744e26ce1249d9ccb23e356d6010679
^ permalink raw reply
* Re: sky2, vlan and nat/masquerading
From: Jesse Gross @ 2011-03-22 2:14 UTC (permalink / raw)
To: Christian Hesse; +Cc: netdev
In-Reply-To: <20110318074612.37cfc0d5@leda.vpn.lugor.de>
On Thu, Mar 17, 2011 at 11:46 PM, Christian Hesse <mail@eworm.de> wrote:
> On Thu, 17 Mar 2011 22:40:44 +0100 Christian Hesse <mail@eworm.de> wrote:
>> On Thu, 17 Mar 2011 12:23:48 -0700 Jesse Gross <jesse@nicira.com> wrote:
>> > Can you try using ethtool to turn off txvlan and see if that makes a
>> > difference?
>>
>> Sure. I will give it a try tomorrow.
>
> No, that does not make a difference.
Hmm, this problem seems very strange to me. I'm not sure that there
is much more I can do without reproducing the problem and it sounds
like I won't be able to do that on my hardware.
If you're able to, the best thing to do would be insert some debugging
statements along the transmit path to find out exactly where the
additional vlan tag is being inserted. Although it only happens with
a single driver, it must be occurring in software because the tag
shows up in tcpdump on the transmit side.
^ permalink raw reply
* Re: ipv6 secondary ips and default ipv6 ip for new outbound connections
From: Brian Haley @ 2011-03-22 2:14 UTC (permalink / raw)
To: Jim Westfall; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <20110319033507.GY13831@surrealistic.net>
On 03/18/2011 11:35 PM, Jim Westfall wrote:
> Brian Haley <brian.haley@hp.com> wrote [03.18.11]:
>> On 03/18/2011 07:03 PM, Jim Westfall wrote:
>>> Hi
>>>
>>> On ipv4 the first ip added to a nic will be used as the source ip for
>>> new outbound connections. Any additional ips, in the same netblock,
>>> will be added as secondaries.
>>>
>>> ipv6 seems to have the opposite behavior. The last ipv6 ip added to a
>>> nic is be used for new outbound connections.
<snip>
>> According to commit 8a6ce0c083f5736e90dabe6d8ce077e7dd0fa35f it's done this
>> way for backward-compatibility - we used to always put new addresses at the
>> front, then we started sorting them by scope. I couldn't find in the archives
>> who needed the backward-compatible behavior (it was way back in 2006), but
>> Yoshifuji proposed it and I Acked it.
>>
>> You could see if this patch helps you out, but I'm not sure if changing this
>> would break someone else, you'd have to see about putting a knob to control
>> this.
>>
>> -Brian
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 3daaf3c..8c7d5a5 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -577,7 +577,7 @@ ipv6_link_dev_addr(struct inet6_dev *idev, struct inet6_ifaddr *ifp)
>> list_for_each(p, &idev->addr_list) {
>> struct inet6_ifaddr *ifa
>> = list_entry(p, struct inet6_ifaddr, if_list);
>> - if (ifp_scope >= ipv6_addr_src_scope(&ifa->addr))
>> + if (ifp_scope > ipv6_addr_src_scope(&ifa->addr))
>> break;
>> }
>>
>
> Hi
>
> Your patch fixes it for me.
Then we need to get Yoshifuji to Ack it since he wanted the previous behavior of
newest-added being first.
-Brian
^ permalink raw reply
* Re: bnx2 vlan issue
From: Jesse Gross @ 2011-03-22 2:05 UTC (permalink / raw)
To: Seblu; +Cc: netdev
In-Reply-To: <AANLkTimJsBYefz3+3HgBgrHUR33g+TWuBp15E=VODohb@mail.gmail.com>
On Thu, Mar 17, 2011 at 4:22 PM, Seblu <seblu@seblu.net> wrote:
> On Thu, Mar 17, 2011 at 8:16 PM, Jesse Gross <jesse@nicira.com> wrote:
>> On Thu, Mar 17, 2011 at 11:02 AM, Seblu <seblu@seblu.net> wrote:
>>> On Thu, Mar 17, 2011 at 3:51 PM, Seblu <seblu@seblu.net> wrote:
>>
>> It was a bug that it worked at all and whether it worked depended on
>> the hardware/firmware/driver. If an interface is attached to a
>> bridge, the bridge takes all the packets received on that interface,
>> including vlan packets.
> ok i see.
>
>>
>> There are a few ways to setup vlans and bridging that work on all
>> kernels and with all NICs:
>>
>> * vlans on interface, bridges contain vlan devices. This gives you a
>> bridge for each vlan.
> Except for untagged isn't it? This is why i fail. I need to have a
> bridge with the untagged vlan from an interface.
> Before 2.6.37 (with most of drivers/firmware/hardware i use) when a
> vlan is defined on an interface, for example eth0, eth0.15 show vlan15
> frames and eth0 show untagged frames.
>
> I've tryed eth0.0, expecting to have untagged vlan, but this doesnt work.
eth0.0 will get packets that have a vlan tag with vid 0, some people
use it as just another vlan.
> How can I create a bridge with the untagged vlan from an interface?
>
>> * bridge on interface, vlans on bridge device. This gives you a
>> bridge with all packets and vlan devices can give you specific vlans.
> I cannot use this schema, i used bridge to bring together vnet
> interface and vlan interface.
I'm not sure I understand why you say you can't use this. You can
combine vlans and bridging pretty much arbitrarily, including stacking
multiple layers.
>
>> * Use ebtables rules in the bridge to accept/reject certain packets as desired.
> I don't see how use ebtables to push untagged frame to a dedicated
> iface which can be added in a bridge.
You could have a bridge on the raw interface and connect all of the
VMs that need untagged traffic. If you add an ebtables rule to reject
tagged traffic then vlan devices on the interface will continue to
work as before.
^ permalink raw reply
* Re: dccp test-tree [RFC] [Patch 1/1] dccp: Only activate NN values after receiving the Confirm option
From: Samuel Jero @ 2011-03-22 1:49 UTC (permalink / raw)
To: Gerrit Renker, dccp, netdev
In-Reply-To: <20110318113052.GA5508@gerrit.erg.abdn.ac.uk>
[-- Attachment #1: Type: text/plain, Size: 4836 bytes --]
> I have revised the patch set addressing the following comments of this thread:
>
> 1) Renamed function to dccp_feat_nn_get().
> 2) Added test to ensure that function is called for NN
> features back in.
> 3) Replaced ccid2_ack_ratio_next() with calls to that
> function (dccp_feat_nn_get(sk, DCCPF_ACK_RATIO)).
> 4) Replaced check for dccps_l_seq_win in ccid2_change_l_seq_window
> with call to dccp_feat_nn_get(sk, DCCPF_SEQUENCE_WINDOW)
> (for similar calls see separate, attached patch).
> 5) De-inlined ccid2_ack_ratio_next() wrapper as discussed.
> 6) Updated remaining patches with regard to 1-4.
Patch Set looks good except for the very last one ("replace remaining
references to local Sequence Window value"). See below for comments
about that patch.
> | > If low_threshold == high_threshold, it oscillates. I think you have already done
> | > some work on this in the code using CCID2_WIN_CHANGE_FACTOR.
> |
> | I'm not entirely certain what low_threshold and high_threshold you are
> | talking about. I have not seen sequence window oscillations other as the
> | congestion window oscillates in congestion avoidance as is expected.
> |
> I meant a Schmitt Trigger like behaviour:
> * low_threshold: to change from lower value to higher value
> * hi_threshold: to revert from higher value back to lower value
> In these electronic devices there is a gap between low and hi, to avoid fluctuations.
> Also audio effect gates have a similar setting.
That makes sense now. Thanks for the explanation. I didn't write the
current code with that in mind. However, it does a good job at avoiding
oscillation most of the time.
Below is the very last patch you added to the test tree. I believe both
of the changes made are not needed. See comments inline.
> dccp ccid-2: replace remaining references to local Sequence Window value
>
> This replaces the remaining references to dccps_l_seq_win with the corresponding
> call to retrieve the current (STABLE if not in negotiation, CHANGING if being
> negotiated) value of the feature-local sequence window.
>
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
> ---
> net/dccp/ccids/ccid2.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> --- a/net/dccp/ccids/ccid2.c
> +++ b/net/dccp/ccids/ccid2.c
> @@ -441,10 +441,10 @@ static void ccid2_new_ack(struct sock *s
> {
> struct ccid2_hc_tx_sock *hc = ccid2_hc_tx_sk(sk);
> struct dccp_sock *dp = dccp_sk(sk);
> + u64 l_seq_win = dccp_feat_nn_get(sk, DCCPF_SEQUENCE_WINDOW);
> int r_seq_used = hc->tx_cwnd / dp->dccps_l_ack_ratio;
>
> - if (hc->tx_cwnd < dp->dccps_l_seq_win &&
> - r_seq_used < dp->dccps_r_seq_win) {
> + if (hc->tx_cwnd < l_seq_win && r_seq_used < dp->dccps_r_seq_win) {
> if (hc->tx_cwnd < hc->tx_ssthresh) {
> if (*maxincr > 0 && ++hc->tx_packets_acked >= 2) {
> hc->tx_cwnd += 1;
In this case I believe we want to wait and use the confirmed sequence
window size (like the unmodified code does).
Using the currently negotiating value could result in a congestion
window much larger than the current sequence window (particularly if the
change below is used). This could cause the validity checking code
(dccp_check_seqno) to reject received packets (prior to the confirm) as
sequence invalid. Further, we essentially move back to changing the
effective sequence window before sending the confirm.
> @@ -466,10 +466,10 @@ static void ccid2_new_ack(struct sock *s
> else if (r_seq_used * CCID2_WIN_CHANGE_FACTOR < dp->dccps_r_seq_win/2)
> ccid2_change_l_ack_ratio(sk, dp->dccps_l_ack_ratio / 2 ? : 1U);
>
> - if (hc->tx_cwnd * CCID2_WIN_CHANGE_FACTOR >= dp->dccps_l_seq_win)
> - ccid2_change_l_seq_window(sk, dp->dccps_l_seq_win * 2);
> - else if (hc->tx_cwnd * CCID2_WIN_CHANGE_FACTOR < dp->dccps_l_seq_win/2)
> - ccid2_change_l_seq_window(sk, dp->dccps_l_seq_win / 2);
> + if (hc->tx_cwnd * CCID2_WIN_CHANGE_FACTOR >= l_seq_win)
> + ccid2_change_l_seq_window(sk, l_seq_win * 2);
> + else if (hc->tx_cwnd * CCID2_WIN_CHANGE_FACTOR < l_seq_win / 2)
> + ccid2_change_l_seq_window(sk, l_seq_win / 2);
>
> /*
> * FIXME: RTT is sampled several times per acknowledgment (for each
Here again, I believe we want to wait and use the confirmed sequence
window size (like the unmodified code does).
Using the currently negotiating value could result in never increasing
the actual sequence window---We ignore confirms for old values so if
begin negotiating a new value before we confirm the previous value we
run the risk of attempting to negotiate a new value several times an
RTT, which results in always receiving (and ignoring) old values.
Samuel Jero
Internetworking Research Group
Ohio University
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH] veth: Fix the byte counters
From: David Miller @ 2011-03-22 1:25 UTC (permalink / raw)
To: ebiederm
Cc: netdev, mirq-linux, eric.dumazet, arnd, greearb, kaber,
daniel.lezcano, xemul
In-Reply-To: <m1oc54i242.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 21 Mar 2011 14:40:29 -0700
>
> Commit 44540960 "veth: move loopback logic to common location" introduced
> a bug in the packet counters. I don't understand why that happened as it
> is not explained in the comments and the mut check in dev_forward_skb
> retains the assumption that skb->len is the total length of the packet.
>
> I just measured this emperically by setting up a veth pair between two
> noop network namespaces setting and attempting a telnet connection between
> the two. I saw three packets in each direction and the byte counters were
> exactly 14*3 = 42 bytes high in each direction. I got the actual
> packet lengths with tcpdump.
>
> So remove the extra ETH_HLEN from the veth byte count totals.
>
> Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net ipv6: Fix duplicate /proc/sys/net/ipv6/neigh directory entries.
From: David Miller @ 2011-03-22 1:23 UTC (permalink / raw)
To: ebiederm; +Cc: netdev, akpm, sunkan
In-Reply-To: <m14o6wjhm5.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 21 Mar 2011 14:20:18 -0700
>
> When I was fixing issues with unregisgtering tables under /proc/sys/net/ipv6/neigh
> by adding a mount point it appears I missed a critical ordering issue, in the
> ipv6 initialization. I had not realized that ipv6_sysctl_register is called
> at the very end of the ipv6 initialization and in particular after we call
> neigh_sysctl_register from ndisc_init.
>
> "neigh" needs to be initialized in ipv6_static_sysctl_register which is
> the first ipv6 table to initialized, and definitely before ndisc_init.
> This removes the weirdness of duplicate tables while still providing a
> "neigh" mount point which prevents races in sysctl unregistering.
>
> This was initially reported at https://bugzilla.kernel.org/show_bug.cgi?id=31232
> Reported-by: sunkan@zappa.cx
> Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] macvlan: Fix use after free of struct macvlan_port.
From: David Miller @ 2011-03-22 1:22 UTC (permalink / raw)
To: ebiederm; +Cc: netdev, kaber, greearb, eric.dumazet
In-Reply-To: <m1aagojhu4.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Mon, 21 Mar 2011 14:15:31 -0700
>
> When the macvlan driver was extended to call unregisgter_netdevice_queue
> in 23289a37e2b127dfc4de1313fba15bb4c9f0cd5b, a use after free of struct
> macvlan_port was introduced. The code in dellink relied on unregister_netdevice
> actually unregistering the net device so it would be safe to free macvlan_port.
>
> Since unregister_netdevice_queue can just queue up the unregister instead of
> performing the unregiser immediately we free the macvlan_port too soon and
> then the code in macvlan_stop removes the macaddress for the set of macaddress
> to listen for and uses memory that has already been freed.
>
> To fix this add a reference count to track when it is safe to free the macvlan_port
> and move the call of macvlan_port_destroy into macvlan_uninit which is guaranteed
> to be called after the final macvlan_port_close.
>
> Signed-off-by: Eric W. Biederman <ebiederm@aristanetworks.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH] fix incorrect spelling in drop monitor protocol
From: David Miller @ 2011-03-22 1:20 UTC (permalink / raw)
To: nhorman; +Cc: netdev
In-Reply-To: <1300739415-26656-1-git-send-email-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 21 Mar 2011 16:30:15 -0400
> It was pointed out to me recently that my spelling could be better :)
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks Neil.
Please start adding some appropriate prefixes to your patch posting
subject lines, I added "net: " to this one, for example.
^ permalink raw reply
* Re: [PATCH] can: c_can: Do basic c_can configuration _before_ enabling the interrupts
From: David Miller @ 2011-03-22 1:19 UTC (permalink / raw)
To: wg; +Cc: jan, bhupesh.sharma, b.spranger, netdev
In-Reply-To: <4D877E61.5080703@grandegger.com>
From: Wolfgang Grandegger <wg@grandegger.com>
Date: Mon, 21 Mar 2011 17:35:45 +0100
> On 03/21/2011 02:47 PM, Jan Altenberg wrote:
>> Hi all,
>>
>> I ran into some trouble while testing the SocketCAN driver for the BOSCH
>> C_CAN controller. The interface is not correctly initialized, if I put
>> some CAN traffic on the line, _while_ the interface is being started
>> (which means: the interface doesn't come up correcty, if there's some RX
>> traffic while doing 'ifconfig can0 up').
>>
>> The current implementation enables the controller interrupts _before_
>> doing the basic c_can configuration. I think, this should be done the
>> other way round.
>>
>> The patch below fixes things for me.
>>
>> Signed-off-by: Jan Altenberg <jan@linutronix.de>
>
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: xfrm: Fix initialize repl field of struct xfrm_state
From: Wei Yongjun @ 2011-03-22 1:04 UTC (permalink / raw)
To: Steffen Klassert; +Cc: David Miller, netdev
In-Reply-To: <20110321120635.GA1290@secunet.com>
> On Mon, Mar 21, 2011 at 05:18:18PM +0800, Wei Yongjun wrote:
>>>> Btw, looking a bit closer to this. I think it would look a bit cleaner
>>>> if we would add the xfrm_init_replay() call to xfrm_init_state() and
>>>> to move the xfrm_init_state() call in xfrm_state_construct() behind
>>>> the assign of the replay settings.
>>> The xfrm_init_replay() should be call after the call to
>>> xfrm_update_ae_params(x, attrs);
>>> since xfrm_update_ae_params() may update the replay_esn.
>>>
>>> So we need move the xfrm_init_state() call just before return x.
>>
>> Oh, sorry, the memcpy looks like dup code since we used
>> kmemdup. It is the same attr XFRMA_REPLAY_ESN_VAL.
>>
> Indeed, we don't need the memcpy here because we do a kmemdup when we
> allocate repay_esn/preplay_esn. But we need to memcpy if we call
> xfrm_update_ae_params() from xfrm_new_ae().
>
> So we could just replace the kmemdup by kmalloc when we allocate
> repay_esn/preplay_esn and move xfrm_init_state() at the end of the
> function, as you suggested. xfrm_update_ae_params() would initialize
> x->replay_esn and x->preplay_esn properly then.
BTW, looking into more about this, another path, XFRM_MSG_NEWAE,
can overwrite the x->replay_esn with the nla_data length, which
may larger then the size we malloc.
>>> The other issue:
>>> static void xfrm_update_ae_params()
>>> {
>>> ...
>>> memcpy(x->replay_esn, replay_esn,
>>> xfrm_replay_state_esn_len(replay_esn));
>>> ...
>>> }
>>>
>>> the memcpy() may cause memory overlap if we build a special
>>> nl_data, we should free it and then do kmemdup()?
>>>
^ permalink raw reply
* Re: [slab poison overwritten] Re: [GIT] Networking
From: Simon Horman @ 2011-03-22 1:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ingo Molnar, David Miller, torvalds, akpm, netdev, linux-kernel,
Peter Zijlstra, Thomas Gleixner, Arnd Bergmann, Pekka Enberg,
Julian Anastasov, Hans Schillstrom
In-Reply-To: <20110322001706.GD27019@verge.net.au>
On Tue, Mar 22, 2011 at 09:17:07AM +0900, Simon Horman wrote:
> On Tue, Mar 22, 2011 at 09:01:33AM +0900, Simon Horman wrote:
> > On Tue, Mar 22, 2011 at 08:29:21AM +0900, Simon Horman wrote:
> > > On Tue, Mar 22, 2011 at 07:13:58AM +0900, Simon Horman wrote:
> > > > On Mon, Mar 21, 2011 at 09:15:40PM +0100, Eric Dumazet wrote:
> > > > > Le lundi 21 mars 2011 à 19:07 +0100, Eric Dumazet a écrit :
> > > > > > Le lundi 21 mars 2011 à 18:39 +0100, Ingo Molnar a écrit :
> > > > > > > here's the same but with kallsyms enabled.
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Ingo
> > > > > > >
> > > > > > > [ 9.585627] initcall 0xffffffff81d5b806 returned 0 after 0 usecs
> > > > > > > [ 9.588960] calling 0xffffffff81d5b9da @ 1
> > > > > > > [ 9.592303] IPVS: Creating netns size=1272 id=0
> > > > > > > [ 9.595646] IPVS: __ip_vs_control_init(): alloc_percpu.
> > > > > > > [ 9.602298] IPVS: cannot register namespace.
> > > > > > > [ 9.605627] IPVS: can't setup control
> > > > > >
> > > > > > It seems IPVS is busted in case of memory allocation error in
> > > > > > __ip_vs_control_init()
> > > > > >
> > > > > > IPVS deinits its "struct netns_ipvs" space, but something (in IPVS) uses
> > > > > > it after free.
> > > > > >
> > > > > > __ip_vs_init() seems to be called before ip_vs_init() completes
> > > > > > correctly. We then keep in net->ipvs a pointer to some freed memory.
> > > > > >
> > > > > > Commit 14e405461e664b7 did some changes in this area
> > > > > >
> > > > > > Simon, any idea ?
> > > > > >
> > > > > >
> > > > >
> > > > > For the time being, we can avoid the false memory allocation error (and
> > > > > leak)
> > > >
> > > > Sorry, that typo is my work.
> > >
> > > With your patch applied I now see the following
> > >
> > > ffff880003bbf1a0 corresponds to &ipvs->app_key in __ip_vs_app_init().
> > > I'll continue looking into this.
> > >
> > > [ 12.610000] IPVS: Creating netns size=2456 id=0
> > > [ 12.630000] IPVS: Registered protocols (TCP, UDP, SCTP, AH, ESP)
> > > [ 12.640000] BUG: key ffff880003bbf1a0 not in .data!
> > > [ 12.640000] ------------[ cut here ]------------
> > > [ 12.640000] WARNING: at kernel/lockdep.c:2701
> > > lockdep_init_map+0x37b/0x570()
> > > [ 12.640000] Hardware name: Bochs
> > > [ 12.640000] Pid: 1, comm: swapper Tainted: G W
> > > 2.6.38-kexec-06330-g69b7efe-dirty #122
> > > [ 12.650000] Call Trace:
> > > [ 12.650000] [<ffffffff8102e685>] warn_slowpath_common+0x75/0xb0
> > > [ 12.650000] [<ffffffff8102e6d5>] warn_slowpath_null+0x15/0x20
> > > [ 12.650000] [<ffffffff8105967b>] lockdep_init_map+0x37b/0x570
> > > [ 12.650000] [<ffffffff8105829d>] ? trace_hardirqs_on+0xd/0x10
> > > [ 12.650000] [<ffffffff81055ad8>] debug_mutex_init+0x38/0x50
> > > [ 12.650000] [<ffffffff8104bc4c>] __mutex_init+0x5c/0x70
> > > [ 12.650000] [<ffffffff81685ee7>] __ip_vs_app_init+0x64/0x86
> > > [ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
> > > [ 12.660000] [<ffffffff811b1c33>] T.620+0x43/0x170
> > > [ 12.660000] [<ffffffff811b1e9a>] ? register_pernet_subsys+0x1a/0x40
> > > [ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
> > > [ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
> > > [ 12.660000] [<ffffffff811b1db7>] register_pernet_operations+0x57/0xb0
> > > [ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
> > > [ 12.670000] [<ffffffff811b1ea9>] register_pernet_subsys+0x29/0x40
> > > [ 12.670000] [<ffffffff81685f19>] ip_vs_app_init+0x10/0x12
> > > [ 12.670000] [<ffffffff81685a87>] ip_vs_init+0x4c/0xff
> > > [ 12.670000] [<ffffffff8166562c>] do_one_initcall+0x7a/0x12e
> > > [ 12.670000] [<ffffffff8166583e>] kernel_init+0x13e/0x1c2
> > > [ 12.670000] [<ffffffff8128c134>] kernel_thread_helper+0x4/0x10
> > > [ 12.670000] [<ffffffff8128ad40>] ? restore_args+0x0/0x30
> > > [ 12.680000] [<ffffffff81665700>] ? kernel_init+0x0/0x1c2
> > > [ 12.680000] [<ffffffff8128c130>] ? kernel_thread_helper+0x0/0x10
> > > [ 12.680000] ---[ end trace 4eaa2a86a8e2da23 ]---
> >
> > It seems that the problem above was introduced by
> > ab8a5e8408c3 ("IPVS: netns awareness to ip_vs_app").
> > I assume the hungs are the cause:
>
> s/hungs/hunks below/
>
> I am a little unsure of what to do about this.
>
> The problem seems to be that ipvs->app_key is not in static storage.
> But I'm not sure how to resolve that given that the struct netns_ipvs is
> per-network namespace. So I guess that the locking needs to be re-worked.
> Again, I'm a little unsure of what the best way forward is.
I had an idea for a fix over breakfast.
IPVS: Use global mutex in ip_vs_app.c
As part of the work to make IPVS network namespace aware
__ip_vs_app_mutex was replaced by a per-namespace lock,
ipvs->app_mutex. ipvs->app_key is also supplied for debugging purposes.
Unfortunately this implementation results in ipvs->app_key residing
in non-static storage which at the very least causes a lockdep warning.
This patch takes the rather heavy-handed approach of reinstating
__ip_vs_app_mutex which will cover access to the ipvs->list_head
of all network namespaces.
[ 12.610000] IPVS: Creating netns size=2456 id=0
[ 12.630000] IPVS: Registered protocols (TCP, UDP, SCTP, AH, ESP)
[ 12.640000] BUG: key ffff880003bbf1a0 not in .data!
[ 12.640000] ------------[ cut here ]------------
[ 12.640000] WARNING: at kernel/lockdep.c:2701 lockdep_init_map+0x37b/0x570()
[ 12.640000] Hardware name: Bochs
[ 12.640000] Pid: 1, comm: swapper Tainted: G W 2.6.38-kexec-06330-g69b7efe-dirty #122
[ 12.650000] Call Trace:
[ 12.650000] [<ffffffff8102e685>] warn_slowpath_common+0x75/0xb0
[ 12.650000] [<ffffffff8102e6d5>] warn_slowpath_null+0x15/0x20
[ 12.650000] [<ffffffff8105967b>] lockdep_init_map+0x37b/0x570
[ 12.650000] [<ffffffff8105829d>] ? trace_hardirqs_on+0xd/0x10
[ 12.650000] [<ffffffff81055ad8>] debug_mutex_init+0x38/0x50
[ 12.650000] [<ffffffff8104bc4c>] __mutex_init+0x5c/0x70
[ 12.650000] [<ffffffff81685ee7>] __ip_vs_app_init+0x64/0x86
[ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
[ 12.660000] [<ffffffff811b1c33>] T.620+0x43/0x170
[ 12.660000] [<ffffffff811b1e9a>] ? register_pernet_subsys+0x1a/0x40
[ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
[ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
[ 12.660000] [<ffffffff811b1db7>] register_pernet_operations+0x57/0xb0
[ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
[ 12.670000] [<ffffffff811b1ea9>] register_pernet_subsys+0x29/0x40
[ 12.670000] [<ffffffff81685f19>] ip_vs_app_init+0x10/0x12
[ 12.670000] [<ffffffff81685a87>] ip_vs_init+0x4c/0xff
[ 12.670000] [<ffffffff8166562c>] do_one_initcall+0x7a/0x12e
[ 12.670000] [<ffffffff8166583e>] kernel_init+0x13e/0x1c2
[ 12.670000] [<ffffffff8128c134>] kernel_thread_helper+0x4/0x10
[ 12.670000] [<ffffffff8128ad40>] ? restore_args+0x0/0x30
[ 12.680000] [<ffffffff81665700>] ? kernel_init+0x0/0x1c2
[ 12.680000] [<ffffffff8128c130>] ? kernel_thread_helper+0x0/0x1global0
Signed-off-by: Simon Horman <horms@verge.net.au>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: Hans Schillstrom <hans@schillstrom.com>
---
include/net/ip_vs.h | 2 --
net/netfilter/ipvs/ip_vs_app.c | 23 ++++++++++-------------
2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 272f593..30b49ed 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -801,8 +801,6 @@ struct netns_ipvs {
struct list_head rs_table[IP_VS_RTAB_SIZE];
/* ip_vs_app */
struct list_head app_list;
- struct mutex app_mutex;
- struct lock_class_key app_key; /* mutex debuging */
/* ip_vs_proto */
#define IP_VS_PROTO_TAB_SIZE 32 /* must be power of 2 */
diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c
index 5c48ffb..2dc6de1 100644
--- a/net/netfilter/ipvs/ip_vs_app.c
+++ b/net/netfilter/ipvs/ip_vs_app.c
@@ -43,6 +43,8 @@ EXPORT_SYMBOL(register_ip_vs_app);
EXPORT_SYMBOL(unregister_ip_vs_app);
EXPORT_SYMBOL(register_ip_vs_app_inc);
+static DEFINE_MUTEX(__ip_vs_app_mutex);
+
/*
* Get an ip_vs_app object
*/
@@ -167,14 +169,13 @@ int
register_ip_vs_app_inc(struct net *net, struct ip_vs_app *app, __u16 proto,
__u16 port)
{
- struct netns_ipvs *ipvs = net_ipvs(net);
int result;
- mutex_lock(&ipvs->app_mutex);
+ mutex_lock(&__ip_vs_app_mutex);
result = ip_vs_app_inc_new(net, app, proto, port);
- mutex_unlock(&ipvs->app_mutex);
+ mutex_unlock(&__ip_vs_app_mutex);
return result;
}
@@ -189,11 +190,11 @@ int register_ip_vs_app(struct net *net, struct ip_vs_app *app)
/* increase the module use count */
ip_vs_use_count_inc();
- mutex_lock(&ipvs->app_mutex);
+ mutex_lock(&__ip_vs_app_mutex);
list_add(&app->a_list, &ipvs->app_list);
- mutex_unlock(&ipvs->app_mutex);
+ mutex_unlock(&__ip_vs_app_mutex);
return 0;
}
@@ -205,10 +206,9 @@ int register_ip_vs_app(struct net *net, struct ip_vs_app *app)
*/
void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app)
{
- struct netns_ipvs *ipvs = net_ipvs(net);
struct ip_vs_app *inc, *nxt;
- mutex_lock(&ipvs->app_mutex);
+ mutex_lock(&__ip_vs_app_mutex);
list_for_each_entry_safe(inc, nxt, &app->incs_list, a_list) {
ip_vs_app_inc_release(net, inc);
@@ -216,7 +216,7 @@ void unregister_ip_vs_app(struct net *net, struct ip_vs_app *app)
list_del(&app->a_list);
- mutex_unlock(&ipvs->app_mutex);
+ mutex_unlock(&__ip_vs_app_mutex);
/* decrease the module use count */
ip_vs_use_count_dec();
@@ -501,7 +501,7 @@ static void *ip_vs_app_seq_start(struct seq_file *seq, loff_t *pos)
struct net *net = seq_file_net(seq);
struct netns_ipvs *ipvs = net_ipvs(net);
- mutex_lock(&ipvs->app_mutex);
+ mutex_lock(&__ip_vs_app_mutex);
return *pos ? ip_vs_app_idx(ipvs, *pos - 1) : SEQ_START_TOKEN;
}
@@ -535,9 +535,7 @@ static void *ip_vs_app_seq_next(struct seq_file *seq, void *v, loff_t *pos)
static void ip_vs_app_seq_stop(struct seq_file *seq, void *v)
{
- struct netns_ipvs *ipvs = net_ipvs(seq_file_net(seq));
-
- mutex_unlock(&ipvs->app_mutex);
+ mutex_unlock(&__ip_vs_app_mutex);
}
static int ip_vs_app_seq_show(struct seq_file *seq, void *v)
@@ -583,7 +581,6 @@ static int __net_init __ip_vs_app_init(struct net *net)
struct netns_ipvs *ipvs = net_ipvs(net);
INIT_LIST_HEAD(&ipvs->app_list);
- __mutex_init(&ipvs->app_mutex, "ipvs->app_mutex", &ipvs->app_key);
proc_net_fops_create(net, "ip_vs_app", 0, &ip_vs_app_fops);
return 0;
}
--
1.7.2.3
^ permalink raw reply related
* Re: [held lock freed] Re: [GIT] Networking
From: David Miller @ 2011-03-22 1:18 UTC (permalink / raw)
To: arnd
Cc: eric.dumazet, mingo, torvalds, akpm, netdev, linux-kernel,
a.p.zijlstra, tglx
In-Reply-To: <201103211622.40851.arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 21 Mar 2011 16:22:40 +0100
> net/appletalk: fix atalk_release use after free
>
> The BKL removal in appletalk introduced a use-after-free problem,
> where atalk_destroy_socket frees a sock, but we still release
> the socket lock on it.
>
> An easy fix is to take an extra reference on the sock and sock_put
> it when returning from atalk_release.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thanks Arnd.
^ permalink raw reply
* Re: [held lock freed] Re: [GIT] Networking
From: David Miller @ 2011-03-22 1:16 UTC (permalink / raw)
To: mingo
Cc: eric.dumazet, torvalds, akpm, netdev, linux-kernel, a.p.zijlstra,
tglx, arnd
In-Reply-To: <20110321161528.GA28580@elte.hu>
From: Ingo Molnar <mingo@elte.hu>
Date: Mon, 21 Mar 2011 17:15:28 +0100
>
> * Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>> Here is a probable fix.
>>
>> [PATCH] ipx: fix ipx_release()
>
> This patch did the trick - thanks Eric!
>
> Tested-by: Ingo Molnar <mingo@elte.hu>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH] snmp: SNMP_UPD_PO_STATS_BH() always called from softirq
From: David Miller @ 2011-03-22 1:13 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1300707055.2884.144.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 21 Mar 2011 12:30:55 +0100
> We dont need to test if we run from softirq context, we definitely are.
>
> This saves few instructions in ip_rcv() & ip_rcv_finish()
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] l2tp: fix possible oops on l2tp_eth module unload
From: David Miller @ 2011-03-22 1:10 UTC (permalink / raw)
To: jchapman; +Cc: netdev
In-Reply-To: <201103210926.p2L9QQBd009047@bert.katalix.com>
From: James Chapman <jchapman@katalix.com>
Date: Mon, 21 Mar 2011 09:26:26 GMT
> A struct used in the l2tp_eth driver for registering network namespace
> ops was incorrectly marked as __net_initdata, leading to oops when
> module unloaded.
...
> Signed-off-by: James Chapman <jchapman@katalix.com>
Applied, thanks James.
> Push to stable trees too?
Yep I'll take care of that.
^ permalink raw reply
* Re: [PATCH v3] xfrm: Fix initialize repl field of struct xfrm_state
From: David Miller @ 2011-03-22 1:08 UTC (permalink / raw)
To: yjwei; +Cc: netdev, steffen.klassert
In-Reply-To: <4D870E2F.50601@cn.fujitsu.com>
From: Wei Yongjun <yjwei@cn.fujitsu.com>
Date: Mon, 21 Mar 2011 16:37:03 +0800
> Commit 'xfrm: Move IPsec replay detection functions to a separate file'
> (9fdc4883d92d20842c5acea77a4a21bb1574b495)
> introduce repl field to struct xfrm_state, and only initialize it
> under SA's netlink create path, the other path, such as pf_key,
> ipcomp/ipcomp6 etc, the repl field remaining uninitialize. So if
> the SA is created by pf_key, any input packet with SA's encryption
> algorithm will cause panic.
I'll apply this, thanks.
I know you guys are discussing alternative ways to handle this,
but getting the crash fixed in the simplest way now doesn't hurt.
^ permalink raw reply
* Re: [slab poison overwritten] Re: [GIT] Networking
From: Simon Horman @ 2011-03-22 0:17 UTC (permalink / raw)
To: Eric Dumazet
Cc: Ingo Molnar, David Miller, torvalds, akpm, netdev, linux-kernel,
Peter Zijlstra, Thomas Gleixner, Arnd Bergmann, Pekka Enberg,
Julian Anastasov, Hans Schillstrom
In-Reply-To: <20110322000130.GC27019@verge.net.au>
On Tue, Mar 22, 2011 at 09:01:33AM +0900, Simon Horman wrote:
> On Tue, Mar 22, 2011 at 08:29:21AM +0900, Simon Horman wrote:
> > On Tue, Mar 22, 2011 at 07:13:58AM +0900, Simon Horman wrote:
> > > On Mon, Mar 21, 2011 at 09:15:40PM +0100, Eric Dumazet wrote:
> > > > Le lundi 21 mars 2011 à 19:07 +0100, Eric Dumazet a écrit :
> > > > > Le lundi 21 mars 2011 à 18:39 +0100, Ingo Molnar a écrit :
> > > > > > here's the same but with kallsyms enabled.
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Ingo
> > > > > >
> > > > > > [ 9.585627] initcall 0xffffffff81d5b806 returned 0 after 0 usecs
> > > > > > [ 9.588960] calling 0xffffffff81d5b9da @ 1
> > > > > > [ 9.592303] IPVS: Creating netns size=1272 id=0
> > > > > > [ 9.595646] IPVS: __ip_vs_control_init(): alloc_percpu.
> > > > > > [ 9.602298] IPVS: cannot register namespace.
> > > > > > [ 9.605627] IPVS: can't setup control
> > > > >
> > > > > It seems IPVS is busted in case of memory allocation error in
> > > > > __ip_vs_control_init()
> > > > >
> > > > > IPVS deinits its "struct netns_ipvs" space, but something (in IPVS) uses
> > > > > it after free.
> > > > >
> > > > > __ip_vs_init() seems to be called before ip_vs_init() completes
> > > > > correctly. We then keep in net->ipvs a pointer to some freed memory.
> > > > >
> > > > > Commit 14e405461e664b7 did some changes in this area
> > > > >
> > > > > Simon, any idea ?
> > > > >
> > > > >
> > > >
> > > > For the time being, we can avoid the false memory allocation error (and
> > > > leak)
> > >
> > > Sorry, that typo is my work.
> >
> > With your patch applied I now see the following
> >
> > ffff880003bbf1a0 corresponds to &ipvs->app_key in __ip_vs_app_init().
> > I'll continue looking into this.
> >
> > [ 12.610000] IPVS: Creating netns size=2456 id=0
> > [ 12.630000] IPVS: Registered protocols (TCP, UDP, SCTP, AH, ESP)
> > [ 12.640000] BUG: key ffff880003bbf1a0 not in .data!
> > [ 12.640000] ------------[ cut here ]------------
> > [ 12.640000] WARNING: at kernel/lockdep.c:2701
> > lockdep_init_map+0x37b/0x570()
> > [ 12.640000] Hardware name: Bochs
> > [ 12.640000] Pid: 1, comm: swapper Tainted: G W
> > 2.6.38-kexec-06330-g69b7efe-dirty #122
> > [ 12.650000] Call Trace:
> > [ 12.650000] [<ffffffff8102e685>] warn_slowpath_common+0x75/0xb0
> > [ 12.650000] [<ffffffff8102e6d5>] warn_slowpath_null+0x15/0x20
> > [ 12.650000] [<ffffffff8105967b>] lockdep_init_map+0x37b/0x570
> > [ 12.650000] [<ffffffff8105829d>] ? trace_hardirqs_on+0xd/0x10
> > [ 12.650000] [<ffffffff81055ad8>] debug_mutex_init+0x38/0x50
> > [ 12.650000] [<ffffffff8104bc4c>] __mutex_init+0x5c/0x70
> > [ 12.650000] [<ffffffff81685ee7>] __ip_vs_app_init+0x64/0x86
> > [ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
> > [ 12.660000] [<ffffffff811b1c33>] T.620+0x43/0x170
> > [ 12.660000] [<ffffffff811b1e9a>] ? register_pernet_subsys+0x1a/0x40
> > [ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
> > [ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
> > [ 12.660000] [<ffffffff811b1db7>] register_pernet_operations+0x57/0xb0
> > [ 12.660000] [<ffffffff81685a3b>] ? ip_vs_init+0x0/0xff
> > [ 12.670000] [<ffffffff811b1ea9>] register_pernet_subsys+0x29/0x40
> > [ 12.670000] [<ffffffff81685f19>] ip_vs_app_init+0x10/0x12
> > [ 12.670000] [<ffffffff81685a87>] ip_vs_init+0x4c/0xff
> > [ 12.670000] [<ffffffff8166562c>] do_one_initcall+0x7a/0x12e
> > [ 12.670000] [<ffffffff8166583e>] kernel_init+0x13e/0x1c2
> > [ 12.670000] [<ffffffff8128c134>] kernel_thread_helper+0x4/0x10
> > [ 12.670000] [<ffffffff8128ad40>] ? restore_args+0x0/0x30
> > [ 12.680000] [<ffffffff81665700>] ? kernel_init+0x0/0x1c2
> > [ 12.680000] [<ffffffff8128c130>] ? kernel_thread_helper+0x0/0x10
> > [ 12.680000] ---[ end trace 4eaa2a86a8e2da23 ]---
>
> It seems that the problem above was introduced by
> ab8a5e8408c3 ("IPVS: netns awareness to ip_vs_app").
> I assume the hungs are the cause:
s/hungs/hunks below/
I am a little unsure of what to do about this.
The problem seems to be that ipvs->app_key is not in static storage.
But I'm not sure how to resolve that given that the struct netns_ipvs is
per-network namespace. So I guess that the locking needs to be re-worked.
Again, I'm a little unsure of what the best way forward is.
> diff --git a/net/netfilter/ipvs/ip_vs_app.c b/net/netfilter/ipvs/ip_vs_app.c
> index 40b09cc..286f465 100644
> --- a/net/netfilter/ipvs/ip_vs_app.c
> +++ b/net/netfilter/ipvs/ip_vs_app.c
> @@ -43,11 +43,6 @@ EXPORT_SYMBOL(register_ip_vs_app);
> EXPORT_SYMBOL(unregister_ip_vs_app);
> EXPORT_SYMBOL(register_ip_vs_app_inc);
>
> -/* ipvs application list head */
> -static LIST_HEAD(ip_vs_app_list);
> -static DEFINE_MUTEX(__ip_vs_app_mutex);
> -
> -
> /*
> * Get an ip_vs_app object
> */
>
> @@ -571,9 +580,13 @@ static const struct file_operations ip_vs_app_fops = {
>
> static int __net_init __ip_vs_app_init(struct net *net)
> {
> + struct netns_ipvs *ipvs = net_ipvs(net);
> +
> if (!net_eq(net, &init_net)) /* netns not enabled yet */
> return -EPERM;
>
> + INIT_LIST_HEAD(&ipvs->app_list);
> + __mutex_init(&ipvs->app_mutex, "ipvs->app_mutex", &ipvs->app_key);
> proc_net_fops_create(net, "ip_vs_app", 0, &ip_vs_app_fops);
> return 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