* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: Jim Baxter @ 2013-02-21 17:07 UTC (permalink / raw)
To: netdev
In-Reply-To: <CAHrpEqQMZNmzRrNQK60hokR2r5S=U65YdC-5h6aHHHzAVdEWKw@mail.gmail.com>
Is the general issue in this driver that the fec_probe function:
request_irq(irq, fec_enet_interrupt, IRQF_DISABLED, pdev->name, ndev);
is called before fec_ptp_init() and fec_enet_init() have been called so that
there is a chance the fec_enet_interrupt can occur before the hardware and data
structures are fully setup?
^ permalink raw reply
* Re: [PATCH 1/4] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
From: Vlad Yasevich @ 2013-02-21 17:18 UTC (permalink / raw)
To: Roberts, Lee A.
Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <D64EC45690EF85409BA6C4730E0162244310DEE7@G4W3231.americas.hpqcorp.net>
On 02/21/2013 11:44 AM, Roberts, Lee A. wrote:
> From: Lee A. Roberts <lee.roberts@hp.com>
>
> Resolve SCTP association hangs observed during SCTP stress
> testing. Observable symptoms include communications hangs
> with data being held in the association lobby (ordering)
> queue. Close examination of reassembly/ordering queues shows
> duplicated packets.
>
> In sctp_tsnmap_grow(), correct off-by-one errors when copying
> and resizing the tsnmap. If max_tsn_seen is in the LSB of the
> word, this bit can be lost, causing the corresponding packet
> to be transmitted again and to be entered as a duplicate into
> the SCTP reassembly/ordering queues.
>
> Patch applies to linux-3.8 kernel.
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> ---
> net/sctp/tsnmap.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-vanilla/net/sctp/tsnmap.c linux-3.8-SCTP+1/net/sctp/tsnmap.c
> --- linux-3.8-vanilla/net/sctp/tsnmap.c 2013-02-18 16:58:34.000000000 -0700
> +++ linux-3.8-SCTP+1/net/sctp/tsnmap.c 2013-02-20 08:01:02.555223259 -0700
> @@ -369,14 +369,15 @@ static int sctp_tsnmap_grow(struct sctp_
> if (gap >= SCTP_TSN_MAP_SIZE)
No that I think about this a bit more, this should be gap + 1. If you
do that, you might as well call sctp_tsnmap_grow() with gap+1 as
argument and then can just use the 'gap' everywhere inside.
> return 0;
>
> - inc = ALIGN((gap - map->len),BITS_PER_LONG) + SCTP_TSN_MAP_INCREMENT;
> + inc = ALIGN((gap - map->len + 1), BITS_PER_LONG)
> + + SCTP_TSN_MAP_INCREMENT;
> len = min_t(u16, map->len + inc, SCTP_TSN_MAP_SIZE);
>
> new = kzalloc(len>>3, GFP_ATOMIC);
> if (!new)
> return 0;
>
> - bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn);
> + bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn + 1);
Can simplify that this by using map->cumulative_tsn_ack_point instead of
base_tsn.
-vlad
> kfree(map->tsn_map);
> map->tsn_map = new;
> map->len = len;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 2/4] sctp: fix association hangs due to reneging packets below the cumulative TSN ACK point
From: Vlad Yasevich @ 2013-02-21 17:19 UTC (permalink / raw)
To: Roberts, Lee A.
Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <D64EC45690EF85409BA6C4730E0162244310DEFE@G4W3231.americas.hpqcorp.net>
On 02/21/2013 11:44 AM, Roberts, Lee A. wrote:
> From: Lee A. Roberts <lee.roberts@hp.com>
>
> Resolve SCTP association hangs observed during SCTP stress
> testing. Observable symptoms include communications hangs
> with data being held in the association reassembly and/or lobby
> (ordering) queues. Close examination of reassembly queue shows
> missing packets.
>
> In sctp_ulpq_renege_list(), do not renege packets below the
> cumulative TSN ACK point.
>
> Patch applies to linux-3.8 kernel.
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/ulpqueue.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP+1/net/sctp/ulpqueue.c linux-3.8-SCTP+2/net/sctp/ulpqueue.c
> --- linux-3.8-SCTP+1/net/sctp/ulpqueue.c 2013-02-18 16:58:34.000000000 -0700
> +++ linux-3.8-SCTP+2/net/sctp/ulpqueue.c 2013-02-21 07:39:40.888281496 -0700
> @@ -969,11 +969,16 @@ static __u16 sctp_ulpq_renege_list(struc
>
> tsnmap = &ulpq->asoc->peer.tsn_map;
>
> - while ((skb = __skb_dequeue_tail(list)) != NULL) {
> - freed += skb_headlen(skb);
> + while ((skb = skb_peek_tail(list)) != NULL) {
> event = sctp_skb2event(skb);
> tsn = event->tsn;
>
> + /* Don't renege below the Cumulative TSN ACK Point. */
> + if (TSN_lte(tsn, sctp_tsnmap_get_ctsn(tsnmap)))
> + break;
> +
> + __skb_unlink(skb, list);
> + freed += skb_headlen(skb);
> sctp_ulpevent_free(event);
> sctp_tsnmap_renege(tsnmap, tsn);
> if (freed >= needed)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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: i.Mx6Quad - eth0: tx queue full!
From: Vikram Narayanan @ 2013-02-21 17:20 UTC (permalink / raw)
To: Frank Li; +Cc: Troy Kisky, netdev, Fabio Estevam
In-Reply-To: <CAHrpEqQc8QGV0UNgY_+-JVVQ8Tfq8gGfy6LfKFeaFNxf7hN7sA@mail.gmail.com>
On 2/18/2013 12:59 PM, Frank Li wrote:
> Do you have a way to duplicate this issue consistently?
I think you meant "reproduce".
Yes. Almost everytime in my hardware.
Any network load would trigger this.
On the NFS, I did something like
for i in {1..10000};do;ls -lR *;done
and did a flood ping to the target as well.
Hope this helps,
Vikram
^ permalink raw reply
* Re: [PATCH 3/4] sctp: fix association hangs due to errors when reneging events from the ordering queue
From: Vlad Yasevich @ 2013-02-21 17:23 UTC (permalink / raw)
To: Roberts, Lee A.
Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <D64EC45690EF85409BA6C4730E0162244310DF0B@G4W3231.americas.hpqcorp.net>
On 02/21/2013 11:44 AM, Roberts, Lee A. wrote:
> From: Lee A. Roberts <lee.roberts@hp.com>
>
> Resolve SCTP association hangs observed during SCTP stress
> testing. Observable symptoms include communications hangs
> with data being held in the association reassembly and/or lobby
> (ordering) queues. Close examination of reassembly queue shows
> missing packets.
>
> In sctp_ulpq_renege_list(), events being reneged from the
> ordering queue may correspond to multiple TSNs. Identify
> all affected packets; sum freed space and renege from the
> tsnmap.
>
> Patch applies to linux-3.8 kernel.
>
> Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/ulpqueue.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-SCTP+2/net/sctp/ulpqueue.c linux-3.8-SCTP+3/net/sctp/ulpqueue.c
> --- linux-3.8-SCTP+2/net/sctp/ulpqueue.c 2013-02-21 07:39:40.888281496 -0700
> +++ linux-3.8-SCTP+3/net/sctp/ulpqueue.c 2013-02-21 07:55:32.817713326 -0700
> @@ -962,8 +962,8 @@ static __u16 sctp_ulpq_renege_list(struc
> struct sk_buff_head *list, __u16 needed)
> {
> __u16 freed = 0;
> - __u32 tsn;
> - struct sk_buff *skb;
> + __u32 tsn, last_tsn;
> + struct sk_buff *skb, *flist, *last;
> struct sctp_ulpevent *event;
> struct sctp_tsnmap *tsnmap;
>
> @@ -977,10 +977,28 @@ static __u16 sctp_ulpq_renege_list(struc
> if (TSN_lte(tsn, sctp_tsnmap_get_ctsn(tsnmap)))
> break;
>
> - __skb_unlink(skb, list);
> + /* Events in ordering queue may have multiple fragments
> + * corresponding to additional TSNs. Sum the total
> + * freed space; find the last TSN.
> + */
> freed += skb_headlen(skb);
> + flist = skb_shinfo(skb)->frag_list;
> + for (last = flist; flist; flist = flist->next) {
> + last = flist;
> + freed += skb_headlen(last);
> + }
> + if (last)
> + last_tsn = sctp_skb2event(last)->tsn;
> + else
> + last_tsn = tsn;
> +
> + /* Unlink the event, then renege all applicable TSNs. */
> + __skb_unlink(skb, list);
> sctp_ulpevent_free(event);
> - sctp_tsnmap_renege(tsnmap, tsn);
> + while (TSN_lte(tsn, last_tsn)) {
> + sctp_tsnmap_renege(tsnmap, tsn);
> + tsn++;
> + }
> if (freed >= needed)
> return freed;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: Fabio Estevam @ 2013-02-21 17:28 UTC (permalink / raw)
To: Jim Baxter; +Cc: netdev, Li Frank-B20596
In-Reply-To: <loom.20130221T175907-606@post.gmane.org>
On Thu, Feb 21, 2013 at 2:07 PM, Jim Baxter <jim_baxter@mentor.com> wrote:
> Is the general issue in this driver that the fec_probe function:
> request_irq(irq, fec_enet_interrupt, IRQF_DISABLED, pdev->name, ndev);
>
> is called before fec_ptp_init() and fec_enet_init() have been called so that
> there is a chance the fec_enet_interrupt can occur before the hardware and data
> structures are fully setup?
The scenario you described is fixed by the patch that Frank proposed at:
http://patchwork.ozlabs.org/patch/222164/
^ permalink raw reply
* Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
From: Sjur Brændeland @ 2013-02-21 17:28 UTC (permalink / raw)
To: Ohad Ben-Cohen
Cc: Dmitry Tarnyagin, netdev, Linus Walleij, Ido Yariv,
linux-kernel@vger.kernel.org, Erwan Yvin, virtualization,
David S. Miller
In-Reply-To: <CAK=Wgba5XkK3c7qh1tYHMGEe7-Qima96uYO4E+uAPcaB8K6bmQ@mail.gmail.com>
Hi Ohad,
> I was wondering - can you please explain your motivation for using
> vringh in caif ?
>
> We have internally discussed supporting multiple remote processors
> talking to each other using rpmsg, and in that scenario using vringh
> can considerably simplifies the solution (no need to decide in advance
> which side is the "guest" and which is the "host"). Was this the
> general incentive in using vringh in caif too or something else?
The motivation for using vringh was to avoid copying buffers
when sending data from the modem to the host. By using
vringh the modem can transfer datagrams to host only by
transfering buffer pointers via the rings.
We use a carveout to declare a memory area that is passed to the
modem internal memory allocator. This way we get both modem
and host to work on the shared memory region.
For TX traffic we use normal virtio queues for the same reason.
TX buffers can be handled without copying as well.
We are seeing a nice performance gain on the modem side after
implementing this.
Host size zero-copy is more tricky. It's hard to handle ownership of buffers,
if the modem crashes. But we might look into this in the future as well.
Regards,
Sjur
^ permalink raw reply
* Re: [PATCH net-next] be2net: Update copyright year
From: David Miller @ 2013-02-21 17:49 UTC (permalink / raw)
To: sarveshwar.bandi; +Cc: netdev, vasundhara.volam
In-Reply-To: <9190fda9-4232-4467-93ea-acbed86c039b@CMEXHTCAS1.ad.emulex.com>
I posted yesterday on netdev that net-next is closed, thanks.
^ permalink raw reply
* Re: [PATCHv2 vringh 1/3] remoteproc: Add support for vringh (Host vrings)
From: Ohad Ben-Cohen @ 2013-02-21 17:55 UTC (permalink / raw)
To: Sjur Brændeland
Cc: Dmitry Tarnyagin, netdev, Linus Walleij, Ido Yariv,
linux-kernel@vger.kernel.org, Erwan Yvin, virtualization,
David S. Miller
In-Reply-To: <CAJK669YHwR=nf2zSQR6buaA-x1JSowJfU16hkz1+N_WLL0qA+A@mail.gmail.com>
Hi Sjur,
On Thu, Feb 21, 2013 at 7:28 PM, Sjur Brændeland <sjurbren@gmail.com> wrote:
> The motivation for using vringh was to avoid copying buffers
> when sending data from the modem to the host.
I may be missing something here, but why do you need vringh for that?
With rpmsg (which uses two regular vrings) both ends send huge amount
of data without copying it - we just send the pointers across.
Thanks,
Ohad.
^ permalink raw reply
* [PATCH 1/4 v2] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
From: Roberts, Lee A. @ 2013-02-21 17:57 UTC (permalink / raw)
To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
From: Lee A. Roberts <lee.roberts@hp.com>
Resolve SCTP association hangs observed during SCTP stress
testing. Observable symptoms include communications hangs
with data being held in the association lobby (ordering)
queue. Close examination of reassembly/ordering queues shows
duplicated packets.
In sctp_tsnmap_mark(), correct off-by-one error when calculating
gap value for tsnmap.
In sctp_tsnmap_grow(), correct off-by-one error when copying
and resizing the tsnmap. If max_tsn_seen is in the LSB of the
word, this bit can be lost, causing the corresponding packet
to be transmitted again and to be entered as a duplicate into
the SCTP reassembly/ordering queues.
Patch applies to linux-3.8 kernel.
Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
---
net/sctp/tsnmap.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-vanilla/net/sctp/tsnmap.c linux-3.8-SCTP+1/net/sctp/tsnmap.c
--- linux-3.8-vanilla/net/sctp/tsnmap.c 2013-02-18 16:58:34.000000000 -0700
+++ linux-3.8-SCTP+1/net/sctp/tsnmap.c 2013-02-21 10:44:15.985075048 -0700
@@ -122,7 +122,7 @@ int sctp_tsnmap_mark(struct sctp_tsnmap
if (TSN_lt(tsn, map->base_tsn))
return 0;
- gap = tsn - map->base_tsn;
+ gap = tsn - map->cumulative_tsn_ack_point;
if (gap >= map->len && !sctp_tsnmap_grow(map, gap))
return -ENOMEM;
@@ -369,14 +369,15 @@ static int sctp_tsnmap_grow(struct sctp_
if (gap >= SCTP_TSN_MAP_SIZE)
return 0;
- inc = ALIGN((gap - map->len),BITS_PER_LONG) + SCTP_TSN_MAP_INCREMENT;
+ inc = ALIGN((gap - map->len), BITS_PER_LONG) + SCTP_TSN_MAP_INCREMENT;
len = min_t(u16, map->len + inc, SCTP_TSN_MAP_SIZE);
new = kzalloc(len>>3, GFP_ATOMIC);
if (!new)
return 0;
- bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn);
+ bitmap_copy(new, map->tsn_map,
+ map->max_tsn_seen - map->cumulative_tsn_ack_point);
kfree(map->tsn_map);
map->tsn_map = new;
map->len = len;
^ permalink raw reply
* Re: why is it not allowed to add a new socket protocol family as an external module?
From: Chris Friesen @ 2013-02-21 17:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Stephen Hemminger, netdev
In-Reply-To: <1361464988.17413.6.camel@edumazet-glaptop>
On 02/21/2013 10:43 AM, Eric Dumazet wrote:
> On Thu, 2013-02-21 at 09:47 -0600, Chris Friesen wrote:
>> The three lock_class_key structs (corresponding to
>> af_family_keys/af_family_slock_keys/af_callback_keys) can be
>> auto-allocated by the network core at dynamic registration time.
>
> Nope, this was the point I specifically raised but you missed it
>
> Take a look at kernel/lockdep.c, lines 2981-2988
>
> All the other stuff you mention seems pretty obvious.
Ah, sorry. I'm not familiar with the guts of lockdep, so I was not
aware that the keys needed to be static.
Given that the lockdep code considers module addresses to be okay, it
should work to make the keys static in the KLM implementing the new
protocol and pass the addresses in at registration time.
Chris
^ permalink raw reply
* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: Jim Baxter @ 2013-02-21 17:59 UTC (permalink / raw)
To: netdev
In-Reply-To: <CAOMZO5AJa7fs+VdA8jGE5mDnxp4ZQ6a9CjbSou5J+RA3uXUjxQ@mail.gmail.com>
Fabio Estevam <festevam <at> gmail.com> writes:
>
> On Thu, Feb 21, 2013 at 2:07 PM, Jim Baxter <jim_baxter <at> mentor.com> wrote:
> > Is the general issue in this driver that the fec_probe function:
> > request_irq(irq, fec_enet_interrupt, IRQF_DISABLED, pdev->name, ndev);
> >
> > is called before fec_ptp_init() and fec_enet_init() have been called so that
> > there is a chance the fec_enet_interrupt can occur before the hardware and
data
> > structures are fully setup?
>
> The scenario you described is fixed by the patch that Frank proposed at:
> http://patchwork.ozlabs.org/patch/222164/
>
That is a good patch that will stop interrupts occurring before request_irq is
called.
What about the problem of fec_enet_interrupt calling fec_ptp_start_cyclecounter
before fec_ptp_init has been called?
Should fec_enet_init and fec_ptp_init be setup before the request_irq is called?
^ permalink raw reply
* RE: [PATCH 1/4] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
From: Roberts, Lee A. @ 2013-02-21 18:00 UTC (permalink / raw)
To: Vlad Yasevich
Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <512656E7.3060908@gmail.com>
Vlad,
> -----Original Message-----
> From: Vlad Yasevich [mailto:vyasevich@gmail.com]
> Sent: Thursday, February 21, 2013 10:19 AM
> To: Roberts, Lee A.
> Cc: linux-sctp@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/4] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
>
> On 02/21/2013 11:44 AM, Roberts, Lee A. wrote:
> > From: Lee A. Roberts <lee.roberts@hp.com>
> >
> > Resolve SCTP association hangs observed during SCTP stress
> > testing. Observable symptoms include communications hangs
> > with data being held in the association lobby (ordering)
> > queue. Close examination of reassembly/ordering queues shows
> > duplicated packets.
> >
> > In sctp_tsnmap_grow(), correct off-by-one errors when copying
> > and resizing the tsnmap. If max_tsn_seen is in the LSB of the
> > word, this bit can be lost, causing the corresponding packet
> > to be transmitted again and to be entered as a duplicate into
> > the SCTP reassembly/ordering queues.
> >
> > Patch applies to linux-3.8 kernel.
> >
> > Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> > ---
> > net/sctp/tsnmap.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-vanilla/net/sctp/tsnmap.c linux-
> 3.8-SCTP+1/net/sctp/tsnmap.c
> > --- linux-3.8-vanilla/net/sctp/tsnmap.c 2013-02-18 16:58:34.000000000 -0700
> > +++ linux-3.8-SCTP+1/net/sctp/tsnmap.c 2013-02-20 08:01:02.555223259 -0700
> > @@ -369,14 +369,15 @@ static int sctp_tsnmap_grow(struct sctp_
> > if (gap >= SCTP_TSN_MAP_SIZE)
>
> No that I think about this a bit more, this should be gap + 1. If you
> do that, you might as well call sctp_tsnmap_grow() with gap+1 as
> argument and then can just use the 'gap' everywhere inside.
I think the calculation of "gap" in sctp_tsnmap_mark() should change:
- gap = tsn - map->base_tsn;
+ gap = tsn - map->cumulative_tsn_ack_point;
>
> > return 0;
> >
> > - inc = ALIGN((gap - map->len),BITS_PER_LONG) + SCTP_TSN_MAP_INCREMENT;
> > + inc = ALIGN((gap - map->len + 1), BITS_PER_LONG)
> > + + SCTP_TSN_MAP_INCREMENT;
> > len = min_t(u16, map->len + inc, SCTP_TSN_MAP_SIZE);
> >
> > new = kzalloc(len>>3, GFP_ATOMIC);
> > if (!new)
> > return 0;
> >
> > - bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn);
> > + bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn + 1);
>
> Can simplify that this by using map->cumulative_tsn_ack_point instead of
> base_tsn.
>
> -vlad
>
I changed the code to use "cumulative_tsn_ack_point" in an updated version
of the patch.
-- Lee
> > kfree(map->tsn_map);
> > map->tsn_map = new;
> > map->len = len;
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
^ permalink raw reply
* Re: [PATCH 1/4 v2] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
From: David Miller @ 2013-02-21 18:02 UTC (permalink / raw)
To: lee.roberts; +Cc: linux-sctp, netdev, linux-kernel
In-Reply-To: <D64EC45690EF85409BA6C4730E0162244310DF62@G4W3231.americas.hpqcorp.net>
From: "Roberts, Lee A." <lee.roberts@hp.com>
Date: Thu, 21 Feb 2013 17:57:46 +0000
When you are given feedback on patches you submit, you should wait
some time for all the feedback to settle, then resubmit your entire
series (not just the patches that needed changes).
> Patch applies to linux-3.8 kernel.
This is not appropriate to mention in a commit message, your patches
might get ported to -stable trees and elsewhere, and such a comment
looks awkward at best in such scenerios.
^ permalink raw reply
* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: David Miller @ 2013-02-21 18:03 UTC (permalink / raw)
To: jim_baxter; +Cc: netdev
In-Reply-To: <loom.20130221T185110-574@post.gmane.org>
From: Jim Baxter <jim_baxter@mentor.com>
Date: Thu, 21 Feb 2013 17:59:23 +0000 (UTC)
> Fabio Estevam <festevam <at> gmail.com> writes:
>
>>
>> On Thu, Feb 21, 2013 at 2:07 PM, Jim Baxter <jim_baxter <at> mentor.com> wrote:
>> > Is the general issue in this driver that the fec_probe function:
>> > request_irq(irq, fec_enet_interrupt, IRQF_DISABLED, pdev->name, ndev);
>> >
>> > is called before fec_ptp_init() and fec_enet_init() have been called so that
>> > there is a chance the fec_enet_interrupt can occur before the hardware and
> data
>> > structures are fully setup?
>>
>> The scenario you described is fixed by the patch that Frank proposed at:
>> http://patchwork.ozlabs.org/patch/222164/
>>
>
> That is a good patch that will stop interrupts occurring before request_irq is
> called.
>
> What about the problem of fec_enet_interrupt calling fec_ptp_start_cyclecounter
> before fec_ptp_init has been called?
>
> Should fec_enet_init and fec_ptp_init be setup before the request_irq is called?
This is a reocurring theme, and there is only one answer.
All software state MUCH be completely, and fully, initialized before
request_irq() is invoked.
There is no other valid way to proceed in this area.
^ permalink raw reply
* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: Fabio Estevam @ 2013-02-21 18:04 UTC (permalink / raw)
To: Jim Baxter; +Cc: netdev, Li Frank-B20596
In-Reply-To: <loom.20130221T185110-574@post.gmane.org>
On Thu, Feb 21, 2013 at 2:59 PM, Jim Baxter <jim_baxter@mentor.com> wrote:
> That is a good patch that will stop interrupts occurring before request_irq is
> called.
>
> What about the problem of fec_enet_interrupt calling fec_ptp_start_cyclecounter
> before fec_ptp_init has been called?
>
> Should fec_enet_init and fec_ptp_init be setup before the request_irq is called?
Yes, I think so. Care to submit a patch?
^ permalink raw reply
* Re: [PATCH 1/1] net: fec: fix crash at mx53 qsb board
From: David Miller @ 2013-02-21 18:04 UTC (permalink / raw)
To: Frank.Li
Cc: lznuaa, linux-arm-kernel, netdev, shawn.guo, B38611, s.hauer,
festevam
In-Reply-To: <1361408848-14118-1-git-send-email-Frank.Li@freescale.com>
From: Frank Li <Frank.Li@freescale.com>
Date: Thu, 21 Feb 2013 09:07:28 +0800
> Unable to handle kernel NULL pointer dereference at virtual address 00000002
> pgd = 80004000
> [00000002] *pgd=00000000
> Internal error: Oops: 5 [#1] SMP ARM
> Modules linked in:
> CPU: 0 Not tainted (3.8.0-rc7-next-20130215+ #346)
> PC is at fec_enet_interrupt+0xd0/0x348
> LR is at fec_enet_interrupt+0xb8/0x348
> pc : [<80372b7c>] lr : [<80372b64>] psr: 60000193
> sp : df855c20 ip : df855c20 fp : df855c74
> r10: 00000516 r9 : 1c000000 r8 : 00000000
> r7 : 00000000 r6 : 00000000 r5 : 00000000 r4 : df9b7800
> r3 : df9b7df4 r2 : 00000000 r1 : 00000000 r0 : df9b7d34
>
> It is possible that issue a irq between request_irq and fec_enet_init.
> Irq handle will be called but driver data structure is not ready yet.
>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
You must fix this by making sure all of the necessary software
datastructures are initialized fully before request_irq() is
invoked.
You absolutely cannot control pending interrupts the chip is already
signalling and which have been queued up in the interrupt controller
already, which will be emitted precisely when you request_irq()
regardless of how you program the chip during this time.
I'm not applying this, fix the bug correctly, thanks.
^ permalink raw reply
* Re: [PATCH] ipv4: fix a bug in ping_err().
From: David Miller @ 2013-02-21 18:08 UTC (permalink / raw)
To: lw; +Cc: netdev
In-Reply-To: <1361441394-16668-1-git-send-email-lw@cn.fujitsu.com>
From: Li Wei <lw@cn.fujitsu.com>
Date: Thu, 21 Feb 2013 18:09:54 +0800
> We should get 'type' and 'code' from the outer ICMP header.
>
> Signed-off-by: Li Wei <lw@cn.fujitsu.com>
> ---
> net/ipv4/ping.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
> index 55c4ee1..2e91006 100644
> --- a/net/ipv4/ping.c
> +++ b/net/ipv4/ping.c
> @@ -322,8 +322,8 @@ void ping_err(struct sk_buff *skb, u32 info)
> struct iphdr *iph = (struct iphdr *)skb->data;
> struct icmphdr *icmph = (struct icmphdr *)(skb->data+(iph->ihl<<2));
> struct inet_sock *inet_sock;
> - int type = icmph->type;
> - int code = icmph->code;
> + int type = icmp_hdr(skb)->type;
> + int code = icmp_hdr(skb)->code;
Isn't that the same thing at this point? skb->data points to the
outer IPv4 header at this point, doesn't it?
^ permalink raw reply
* [PATCH] sunrpc/auth_gss: fix sparse warnings for gss_mech_switch
From: Silviu-Mihai Popescu @ 2013-02-21 18:10 UTC (permalink / raw)
To: netdev; +Cc: Trond.Myklebust, bfields, davem, linux-nfs, linux-kernel
This fixes the following sparse warnings:
* net/sunrpc/auth_gss/gss_mech_switch.c:143:21: warning: symbol
'_gss_mech_get_by_name' was not declared. Should it be static?
* net/sunrpc/auth_gss/gss_mech_switch.c:208:21: warning: symbol
'_gss_mech_get_by_pseudoflavor' was not declared. Should it be static?
Signed-off-by: Silviu-Mihai Popescu <silviupopescu1990@gmail.com>
---
net/sunrpc/auth_gss/gss_mech_switch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c
index b174fcd..f0f4eee 100644
--- a/net/sunrpc/auth_gss/gss_mech_switch.c
+++ b/net/sunrpc/auth_gss/gss_mech_switch.c
@@ -140,7 +140,7 @@ gss_mech_get(struct gss_api_mech *gm)
EXPORT_SYMBOL_GPL(gss_mech_get);
-struct gss_api_mech *
+static struct gss_api_mech *
_gss_mech_get_by_name(const char *name)
{
struct gss_api_mech *pos, *gm = NULL;
@@ -205,7 +205,7 @@ mech_supports_pseudoflavor(struct gss_api_mech *gm, u32 pseudoflavor)
return 0;
}
-struct gss_api_mech *_gss_mech_get_by_pseudoflavor(u32 pseudoflavor)
+static struct gss_api_mech *_gss_mech_get_by_pseudoflavor(u32 pseudoflavor)
{
struct gss_api_mech *gm = NULL, *pos;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] sunrpc/auth_gss: fix sparse warnings for gss_mech_switch
From: Myklebust, Trond @ 2013-02-21 18:15 UTC (permalink / raw)
To: Silviu-Mihai Popescu
Cc: netdev@vger.kernel.org, bfields@fieldses.org, davem@davemloft.net,
linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1361470206-24116-1-git-send-email-silviupopescu1990@gmail.com>
On Thu, 2013-02-21 at 20:10 +0200, Silviu-Mihai Popescu wrote:
> This fixes the following sparse warnings:
> * net/sunrpc/auth_gss/gss_mech_switch.c:143:21: warning: symbol
> '_gss_mech_get_by_name' was not declared. Should it be static?
> * net/sunrpc/auth_gss/gss_mech_switch.c:208:21: warning: symbol
> '_gss_mech_get_by_pseudoflavor' was not declared. Should it be static?
>
> Signed-off-by: Silviu-Mihai Popescu <silviupopescu1990@gmail.com>
> ---
> net/sunrpc/auth_gss/gss_mech_switch.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/gss_mech_switch.c b/net/sunrpc/auth_gss/gss_mech_switch.c
> index b174fcd..f0f4eee 100644
> --- a/net/sunrpc/auth_gss/gss_mech_switch.c
> +++ b/net/sunrpc/auth_gss/gss_mech_switch.c
> @@ -140,7 +140,7 @@ gss_mech_get(struct gss_api_mech *gm)
>
> EXPORT_SYMBOL_GPL(gss_mech_get);
>
> -struct gss_api_mech *
> +static struct gss_api_mech *
> _gss_mech_get_by_name(const char *name)
> {
> struct gss_api_mech *pos, *gm = NULL;
> @@ -205,7 +205,7 @@ mech_supports_pseudoflavor(struct gss_api_mech *gm, u32 pseudoflavor)
> return 0;
> }
>
> -struct gss_api_mech *_gss_mech_get_by_pseudoflavor(u32 pseudoflavor)
> +static struct gss_api_mech *_gss_mech_get_by_pseudoflavor(u32 pseudoflavor)
> {
> struct gss_api_mech *gm = NULL, *pos;
>
This is identical to commit c5f5e9c5d2e9178fb0bfe4f44f0afcc8ad6488ef
(SUNRPC: Add missing static declaration to _gss_mech_get_by_name) which
is already upstream.
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@netapp.com
www.netapp.com
^ permalink raw reply
* Re: [PATCH net-next 2/3] tipc: byte-based overload control on socket receive queue
From: Neil Horman @ 2013-02-21 18:16 UTC (permalink / raw)
To: Jon Maloy; +Cc: Paul Gortmaker, David Miller, netdev, Ying Xue, Jon Maloy
In-Reply-To: <51265134.5080001@ericsson.com>
On Thu, Feb 21, 2013 at 05:54:12PM +0100, Jon Maloy wrote:
> On 02/21/2013 04:07 PM, Neil Horman wrote:
> > On Thu, Feb 21, 2013 at 11:24:19AM +0100, Jon Maloy wrote:
> >> On 02/19/2013 10:44 PM, Neil Horman wrote:
> >>> On Tue, Feb 19, 2013 at 09:16:40PM +0100, Jon Maloy wrote:
> >>>> On 02/19/2013 08:18 PM, Neil Horman wrote:
> >>>>> On Tue, Feb 19, 2013 at 06:54:14PM +0100, Jon Maloy wrote:
> >>>>>> On 02/19/2013 03:26 PM, Neil Horman wrote:
> >>>>>>> On Tue, Feb 19, 2013 at 09:07:54AM +0100, Jon Maloy wrote:
> >>>>>>>> On 02/18/2013 09:47 AM, Neil Horman wrote:
> >>>>>>>>> On Fri, Feb 15, 2013 at 05:57:46PM -0500, Paul Gortmaker wrote:
> >>>>>>>>>> From: Ying Xue <ying.xue@windriver.com>
> >>>> <snip>
> >>>
> >>>>>>
> >>>>>> There are two reasons for this.
> >>>>>> The first one due to the message oriented nature of the flow control for
> >>>>>> connections. Max message size is 65k, and max number of unacked messages
> >>>>>> (at socket level, that is) before the sending process will take a break
> >>>>>> is 1024.
> >>>>>> So, simple maths gives that we must allow for 64MB + sk_overhead to guarantee
> >>>>>> that a connection never is broken because of receiver overload. Contrary to TCP,
> >>>>> Note, this is false, due to the fact that, it presumes that the sender will
> >>>>> honor the congestion window. Granted, that would be a sender violation, but its
> >>>>> still possible for packets to get lost due to receiver overrun.
> >>>>
> >>>> The reason for this high limit is exactly to guard against crazy or malevolent
> >>>> senders. If they respect their send window they will never hit this limit on
> >>>> connections.
> >>>>
> >>> Nope, You don't get to have it both ways - If a sender is malevolent or crazy,
> >>> what makes you think it will respect its send window?
> >>
> >> Nothing. That is why we need this extra security measure that this limit provides.
> >> The normal senders will never hit it, the crazy ones will, and have their
> >> connections broken as a consequence.
> >> Sorry I didn't express this clearly enough.
> > This isn't a security measure. If a sender is well
> > behaved, then as you say, they won't violate their send window, and will
> > ostensibly behave properly (regardless of what limit you set on sk_rcvbuf, be it
> > the large one you currently employ, or something bigger or smaller). If they
> > are malicious, then they will hit the limit, and they won't care (because
> > they're malicious). They may have their connections broken, but so what? Its the
> > receiver that needs to be able to protect itself. The static limit you set using
> > this implementation is likely fine for many systems that have sufficient memory,
> > but consider a system that is memory constrained.
>
> The default limit is insanely high of course, but until we have introduced byte
> oriented flow control this is what we have to deal with, for reasons I explained
> earlier.
>
> If anybody want to turn it down, it will probably work well in most cases.
> The risk of having connections broken rises from 0 to some low percentage.
> If somebody decides to do this, it is probably because he can accept
> that risk, or because he knows the apps he is running are nice, e.g. never send
> out streams of 64k messages the way I described as a worst-case scenario.
>
> > If a user administratively
> > tunes down sk_rcvbuf to avoid over-consumption of memory on a TIPC socket, that
> > condition will be ignored (or at least severly violated). Thats bad, and may
> > lead to memory exhaustion despite having administratively taken action to fix
> > it.
> >
> [...]
>
> > Clearly sequential packet delivery implies the need to queue and postpone
> > application delivery of packets behind the reception of a lost packet in
> > sequence if it is dropped. The problem I'm referring to here is that you seem
> > to have built TIPC around the notion of a single sequence namespace per system,
> > rather than one per socket.
>
> This is the way TIPC and other link-oriented protocols, e.g. LINX, are designed,
> and they have this well-known flip-side. They were never intended to become
> another TCP.
> Many users accept this, and value that the advantages you get from this design
> more than outweigh the disadvantages. If they don't, then they should use TCP
> or SCTP instead.
>
> Anyway the design doesn't by necessity mean that we are bound to violate the
> system limits; I think we have established that now.
>
Agreed, you're not bound to deterministically violate the system limits - my
only request is that you honor them should you reach them.
> >> I could even show that your proposal would cause almost immediate
> >> deadlock, due to the order incoming and outgoing data paths are grabbing
> >> the locks. But I would rather drop this part of the discussion; we can
> >> achieve what we want anyway, with much simpler means. See below.
> >>
> > Perhaps you could, but if you were to deomonstrate how lowering your recieve
> > buffer limit (or even just honoring whatever is set), results in a deadlock
> > (ostensibly within the TIPC protocol code), how do you justify that as a reason
> > why you should be allowed to simply violate your set socket limits, rather than
> > recognizing the deadlock as a coding bug that needs to be fixed? If you like,
> > we can begin a separate thread on that subject and look into fixing the issue.
>
> I wouldn't call it a bug, because it doesn't cause deadlock in the current code,
> but it is clearly a design that can be improved.
I don't understand this - Above you said you could demonstrate how my proposal
(which was to drop packets when they surpassed the sk_rcvbuf limit), would cause
deadlock - if that happens, you have a locking bug. If the only reason this
does not happen currently is because you allow for a large overrun of your
set sk_rcvbuf, then ostensibly a lockup can still be triggered if you have a
misbehaving sender that is willing to send frames past its congestion window.
So I think the root question here is: Does the code currently deadlock if you
drop frames in the receive path? If not, then we should be ok, regardless of
what the rcvbuf value is set to. If so, then we have a bug that needs fixing.
> We do have plans and prototypes ready to greatly simplify the locking structure,
> e.g. by getting rid of the port lock and replace most of the other ones with
> RCU-locks.
> There are however a few other changes we need to get in place first, notably to
> remove the remnants of the native interface, to make that possible. Stay tuned.
>
Okey dokey.
> >>
> > As I noted above, as long as there is a method to change the administrative
> > default, thats fine.
> >
> >> 2: Datagram messages are checked against fractions of this value, according to
> >> their importance priority. E.g. LOW->sk_rcvbuf/16, MEDIUM->sk_rcvbuf/8
> >> HIGH->sk_rcvbuf/4 and CRITICAL->sk_rcvbuf/2.
> >>
> > Yes, this is what I proposed in my initial email.
> >
> >> 3: In the unlikely event that anybody wants to change these limits, he can change
> >> sk_rcvbuf via setsockopt(SOL_SOCK) either by first changing
> >> /proc/sys/net/core/rmem_max as you suggest, or via a dedicated setsockopt(SOL_TIPC).
> >> The latter option would have the advantage that it could enforce a lower limit of
> >> skr_rcvbuf of 64MB, something that would be unthinkable with SOL_SOCK, because it
> >> would obviously affect other protocols.
> > Theres really no need for a private TIPC socket option here. You do this at the
> > socket level, and if you want to set it higher than rmem_max, you adjust
> > rmem_max first. Its just the way the system works. Every other protocol
> > conforms to it, theres no reason TIPC shouldn't also.
> >>
> >> What do you think?
> >>
> > Yes, this is for the most part, good.
>
> Happy to hear that. So we'll go ahead and make this change.
>
> Thank you for your feedback.
Sounds good, thank you!
Neil
> ///jon
>
> > Thanks
> > Neil
> >> ///jon
> >>
> >> If you set:
> >>>
> >>> /proc/sys/net/core/rmem_default
> >>
> >> But this will
> >>>
> >>> to the value that you want all your sockets to have, any socket that gets
> >>> initalized with sock_init_data will inherit that value. Note that, when doing
> >>> so, you may also have to set:
> >>>
> >>>
> >>>
> >>> As you can't adminstratively set your default socket rcvbuf value to something
> >>> larger than the maximum allowed value without raising the maximum allowed value
> >>> first.
> >>>
> >>> Then all you have to do is make sure those values are set during boot up, and for
> >>> users, it will appear as though nothing has changed.
> >>>
> >>> Neil
> >>>
> >>>>
> >>
> >>
>
>
^ permalink raw reply
* [PATCH 1/4 v3] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
From: Roberts, Lee A. @ 2013-02-21 18:23 UTC (permalink / raw)
To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
From: Lee A. Roberts <lee.roberts@hp.com>
Resolve SCTP association hangs observed during SCTP stress
testing. Observable symptoms include communications hangs
with data being held in the association lobby (ordering)
queue. Close examination of reassembly/ordering queues shows
duplicated packets.
In sctp_tsnmap_mark(), correct off-by-one error when calculating
gap value for tsnmap.
In sctp_tsnmap_grow(), correct off-by-one error when copying
and resizing the tsnmap. If max_tsn_seen is in the LSB of the
word, this bit can be lost, causing the corresponding packet
to be transmitted again and to be entered as a duplicate into
the SCTP reassembly/ordering queues.
Patch applies to linux-3.8 kernel.
Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
---
net/sctp/tsnmap.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-vanilla/net/sctp/tsnmap.c linux-3.8-SCTP+1/net/sctp/tsnmap.c
--- linux-3.8-vanilla/net/sctp/tsnmap.c 2013-02-18 16:58:34.000000000 -0700
+++ linux-3.8-SCTP+1/net/sctp/tsnmap.c 2013-02-21 11:16:01.843233297 -0700
@@ -124,7 +124,7 @@ int sctp_tsnmap_mark(struct sctp_tsnmap
gap = tsn - map->base_tsn;
- if (gap >= map->len && !sctp_tsnmap_grow(map, gap))
+ if (gap >= map->len && !sctp_tsnmap_grow(map, gap + 1))
return -ENOMEM;
if (!sctp_tsnmap_has_gap(map) && gap == 0) {
@@ -369,14 +369,15 @@ static int sctp_tsnmap_grow(struct sctp_
if (gap >= SCTP_TSN_MAP_SIZE)
return 0;
- inc = ALIGN((gap - map->len),BITS_PER_LONG) + SCTP_TSN_MAP_INCREMENT;
+ inc = ALIGN((gap - map->len), BITS_PER_LONG) + SCTP_TSN_MAP_INCREMENT;
len = min_t(u16, map->len + inc, SCTP_TSN_MAP_SIZE);
new = kzalloc(len>>3, GFP_ATOMIC);
if (!new)
return 0;
- bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn);
+ bitmap_copy(new, map->tsn_map,
+ map->max_tsn_seen - map->cumulative_tsn_ack_point);
kfree(map->tsn_map);
map->tsn_map = new;
map->len = len;
^ permalink raw reply
* RE: [PATCH 1/4] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
From: Roberts, Lee A. @ 2013-02-21 18:25 UTC (permalink / raw)
To: Vlad Yasevich
Cc: linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <512656E7.3060908@gmail.com>
Vlad,
> -----Original Message-----
> From: Roberts, Lee A.
> Sent: Thursday, February 21, 2013 11:00 AM
> To: 'Vlad Yasevich'
> Cc: linux-sctp@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH 1/4] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
>
> Vlad,
>
> > -----Original Message-----
> > From: Vlad Yasevich [mailto:vyasevich@gmail.com]
> > Sent: Thursday, February 21, 2013 10:19 AM
> > To: Roberts, Lee A.
> > Cc: linux-sctp@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH 1/4] sctp: fix association hangs due to off-by-one errors in sctp_tsnmap_grow()
> >
> > On 02/21/2013 11:44 AM, Roberts, Lee A. wrote:
> > > From: Lee A. Roberts <lee.roberts@hp.com>
> > >
> > > Resolve SCTP association hangs observed during SCTP stress
> > > testing. Observable symptoms include communications hangs
> > > with data being held in the association lobby (ordering)
> > > queue. Close examination of reassembly/ordering queues shows
> > > duplicated packets.
> > >
> > > In sctp_tsnmap_grow(), correct off-by-one errors when copying
> > > and resizing the tsnmap. If max_tsn_seen is in the LSB of the
> > > word, this bit can be lost, causing the corresponding packet
> > > to be transmitted again and to be entered as a duplicate into
> > > the SCTP reassembly/ordering queues.
> > >
> > > Patch applies to linux-3.8 kernel.
> > >
> > > Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> > > ---
> > > net/sctp/tsnmap.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff -uprN -X linux-3.8-vanilla/Documentation/dontdiff linux-3.8-vanilla/net/sctp/tsnmap.c linux-
> > 3.8-SCTP+1/net/sctp/tsnmap.c
> > > --- linux-3.8-vanilla/net/sctp/tsnmap.c 2013-02-18 16:58:34.000000000 -0700
> > > +++ linux-3.8-SCTP+1/net/sctp/tsnmap.c 2013-02-20 08:01:02.555223259 -0700
> > > @@ -369,14 +369,15 @@ static int sctp_tsnmap_grow(struct sctp_
> > > if (gap >= SCTP_TSN_MAP_SIZE)
> >
> > No that I think about this a bit more, this should be gap + 1. If you
> > do that, you might as well call sctp_tsnmap_grow() with gap+1 as
> > argument and then can just use the 'gap' everywhere inside.
>
> I think the calculation of "gap" in sctp_tsnmap_mark() should change:
>
> - gap = tsn - map->base_tsn;
> + gap = tsn - map->cumulative_tsn_ack_point;
>
Oops, this breaks the logic that follows. Using "gap + 1" in the call should work.
I sent an updated patch (v3).
- Lee
> >
> > > return 0;
> > >
> > > - inc = ALIGN((gap - map->len),BITS_PER_LONG) + SCTP_TSN_MAP_INCREMENT;
> > > + inc = ALIGN((gap - map->len + 1), BITS_PER_LONG)
> > > + + SCTP_TSN_MAP_INCREMENT;
> > > len = min_t(u16, map->len + inc, SCTP_TSN_MAP_SIZE);
> > >
> > > new = kzalloc(len>>3, GFP_ATOMIC);
> > > if (!new)
> > > return 0;
> > >
> > > - bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn);
> > > + bitmap_copy(new, map->tsn_map, map->max_tsn_seen - map->base_tsn + 1);
> >
> > Can simplify that this by using map->cumulative_tsn_ack_point instead of
> > base_tsn.
> >
> > -vlad
> >
>
> I changed the code to use "cumulative_tsn_ack_point" in an updated version
> of the patch.
>
> -- Lee
>
> > > kfree(map->tsn_map);
> > > map->tsn_map = new;
> > > map->len = len;
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
^ permalink raw reply
* Re: [PATCH 1/1] net: fec: fix miss init spinlock
From: Fabio Estevam @ 2013-02-21 19:04 UTC (permalink / raw)
To: Jim Baxter; +Cc: netdev, Li Frank-B20596
In-Reply-To: <CAOMZO5DqzNUvfmRxPZAhk4i_+SKbTthHqTkQCvW7K+8o7b42zg@mail.gmail.com>
On Thu, Feb 21, 2013 at 3:04 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Thu, Feb 21, 2013 at 2:59 PM, Jim Baxter <jim_baxter@mentor.com> wrote:
>
>> That is a good patch that will stop interrupts occurring before request_irq is
>> called.
>>
>> What about the problem of fec_enet_interrupt calling fec_ptp_start_cyclecounter
>> before fec_ptp_init has been called?
>>
>> Should fec_enet_init and fec_ptp_init be setup before the request_irq is called?
>
> Yes, I think so. Care to submit a patch?
Ok, I prepared a patch with this fix and will submit shortly.
^ permalink raw reply
* [PATCH] net: fec: Ensure that initialization is done prior to request_irq()
From: Fabio Estevam @ 2013-02-21 19:21 UTC (permalink / raw)
To: davem; +Cc: Frank.Li, s.hauer, jim_baxter, netdev, Fabio Estevam
Currently request_irq() is called prior to fec_enet_init() and fec_ptp_init(),
which causes the following crash on a mx53qsb:
Unable to handle kernel NULL pointer dereference at virtual address 00000002
pgd = 80004000
[00000002] *pgd=00000000
Internal error: Oops: 5 [#1] SMP ARM
Modules linked in:
CPU: 0 Not tainted (3.8.0-rc7-next-20130215+ #346)
PC is at fec_enet_interrupt+0xd0/0x348
LR is at fec_enet_interrupt+0xb8/0x348
pc : [<80372b7c>] lr : [<80372b64>] psr: 60000193
sp : df855c20 ip : df855c20 fp : df855c74
r10: 00000516 r9 : 1c000000 r8 : 00000000
r7 : 00000000 r6 : 00000000 r5 : 00000000 r4 : df9b7800
r3 : df9b7df4 r2 : 00000000 r1 : 00000000 r0 : df9b7d34
Ensure that such initialization functions are called prior to requesting the
interrupts, so that all necessary the data structures are in place when the
irqs occur.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 29d82cf..2dbb36c 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1782,6 +1782,15 @@ fec_probe(struct platform_device *pdev)
fep->phy_interface = ret;
}
+ fep->bufdesc_ex =
+ pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX;
+ if (fep->bufdesc_ex)
+ fec_ptp_init(ndev, pdev);
+
+ ret = fec_enet_init(ndev);
+ if (ret)
+ goto failed_init;
+
for (i = 0; i < FEC_IRQ_NUM; i++) {
irq = platform_get_irq(pdev, i);
if (irq < 0) {
@@ -1819,8 +1828,6 @@ fec_probe(struct platform_device *pdev)
}
fep->clk_ptp = devm_clk_get(&pdev->dev, "ptp");
- fep->bufdesc_ex =
- pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX;
if (IS_ERR(fep->clk_ptp)) {
ret = PTR_ERR(fep->clk_ptp);
fep->bufdesc_ex = 0;
@@ -1843,13 +1850,6 @@ fec_probe(struct platform_device *pdev)
fec_reset_phy(pdev);
- if (fep->bufdesc_ex)
- fec_ptp_init(ndev, pdev);
-
- ret = fec_enet_init(ndev);
- if (ret)
- goto failed_init;
-
ret = fec_enet_mii_init(pdev);
if (ret)
goto failed_mii_init;
@@ -1866,7 +1866,6 @@ fec_probe(struct platform_device *pdev)
failed_register:
fec_enet_mii_remove(fep);
failed_mii_init:
-failed_init:
failed_regulator:
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
@@ -1881,6 +1880,7 @@ failed_clk:
}
failed_irq:
iounmap(fep->hwp);
+failed_init:
failed_ioremap:
free_netdev(ndev);
failed_alloc_etherdev:
@@ -1899,17 +1899,17 @@ fec_drv_remove(struct platform_device *pdev)
unregister_netdev(ndev);
fec_enet_mii_remove(fep);
- for (i = 0; i < FEC_IRQ_NUM; i++) {
- int irq = platform_get_irq(pdev, i);
- if (irq > 0)
- free_irq(irq, ndev);
- }
del_timer_sync(&fep->time_keep);
clk_disable_unprepare(fep->clk_ptp);
if (fep->ptp_clock)
ptp_clock_unregister(fep->ptp_clock);
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
+ for (i = 0; i < FEC_IRQ_NUM; i++) {
+ int irq = platform_get_irq(pdev, i);
+ if (irq > 0)
+ free_irq(irq, ndev);
+ }
iounmap(fep->hwp);
free_netdev(ndev);
--
1.7.9.5
^ 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