| Message ID | 9F4C7D19E8361D4C94921B95BE08B81BA08F83@zin33exm22.fsl.freescale.net |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 21 Dec 09 14:19, Kumar Gopalpet-B05799 wrote: > Michael, > > Kindly try the following changes at your side and let me know the > results. I couldn't test this > as I don't have any TSEC device with me. <snip> Your patch did not apply to vanilla 2.6.32-rc1 at all. I patched the code myself and up to now I do not see any error messages. Uptime is at 33 minutes, without the patch the error message appeared immediately after booting. Just for my understanding. Is the first diff part of the fix as well? With this change the FCB is only removed if there are padded bytes, why would this make a difference here? Kind regards, Michael -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>-----Original Message----- >From: Michael Guntsche [mailto:mike@it-loops.com] >Sent: Monday, December 21, 2009 10:09 PM >To: Kumar Gopalpet-B05799 >Cc: netdev >Subject: Re: [BUG] 2.6.33-rc1 gianfar error message > >On 21 Dec 09 14:19, Kumar Gopalpet-B05799 wrote: >> Michael, >> >> Kindly try the following changes at your side and let me know the >> results. I couldn't test this as I don't have any TSEC >device with me. ><snip> > >Your patch did not apply to vanilla 2.6.32-rc1 at all. I patched the >code myself and up to now I do not see any error messages. Uptime is at >33 minutes, without the patch the error message appeared immediately >after booting. > I am sorry about that, I forgot to mention that my patch was against net-next-2.6 tree. >Just for my understanding. Is the first diff part of the fix as well? >With this change the FCB is only removed if there are padded bytes, why >would this make a difference here? > Yes, the first diff portion is not required. Also, if you confirm that the changes are working fine. I will generate a clean patch and send it out. Thanks for reporting it and testing it out. -- Thanks Sandeep -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 22 Dec 09 10:32, Kumar Gopalpet-B05799 wrote: > >Just for my understanding. Is the first diff part of the fix as well? > >With this change the FCB is only removed if there are padded bytes, why > >would this make a difference here? > > > > Yes, the first diff portion is not required. > > Also, if you confirm that the changes are working fine. > I will generate a clean patch and send it out. > Thanks for reporting it and testing it out. Hi, I have now an uptime of 20 hours with no error messages so far. I had one ppp disconnect but I think this was ADSL related. I will continue running this kernel for now to test it some more but I think the initial problem I was seeing is fixed now. Kind regards, Michael -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2009.12.22 10:32:49 , Kumar Gopalpet-B05799 wrote: > >Just for my understanding. Is the first diff part of the fix as well? > >With this change the FCB is only removed if there are padded bytes, why > >would this make a difference here? > > > > Yes, the first diff portion is not required. > > Also, if you confirm that the changes are working fine. > I will generate a clean patch and send it out. > Thanks for reporting it and testing it out. Hi, Since you said that the first patch is not needed @@ -2470,10 +2470,11 @@ static int gfar_process_frame(struct net_device fcb = (struct rxfcb *)skb->data; /* Remove the FCB from the skb */ - skb_set_queue_mapping(skb, fcb->rq); /* Remove the padded bytes, if there are any */ - if (amount_pull) + if (amount_pull) { + skb_set_queue_mapping(skb, fcb->rq); skb_pull(skb, amount_pull); + } if (priv->rx_csum_enable) gfar_rx_checksum(skb, fcb); I only applied the second one and tested again. Right after the reboot I got an error so apparently this change IS required as well. Kind regards, Michael -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2009.12.22 19:10:00 , Michael Guntsche wrote: > Since you said that the first patch is not needed > > @@ -2470,10 +2470,11 @@ static int gfar_process_frame(struct net_device > fcb = (struct rxfcb *)skb->data; > > /* Remove the FCB from the skb */ > - skb_set_queue_mapping(skb, fcb->rq); > /* Remove the padded bytes, if there are any */ > - if (amount_pull) > + if (amount_pull) { > + skb_set_queue_mapping(skb, fcb->rq); > skb_pull(skb, amount_pull); > + } > > if (priv->rx_csum_enable) > gfar_rx_checksum(skb, fcb); > > I only applied the second one and tested again. Right after the reboot > I got an error so apparently this change IS required as well. I tested this now in the opposite direction and apparently ONLY this patch is needed. I commented out the second diff and did not get any errors so far. Sorry for not testing this before sending my previous mail. Kind regards, Michael -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>-----Original Message----- >From: Michael Guntsche [mailto:mike@it-loops.com] >Sent: Tuesday, December 22, 2009 11:58 PM >To: Kumar Gopalpet-B05799 >Cc: netdev >Subject: Re: [BUG] 2.6.33-rc1 gianfar error message > >On 2009.12.22 19:10:00 , Michael Guntsche wrote: >> Since you said that the first patch is not needed >> >> @@ -2470,10 +2470,11 @@ static int gfar_process_frame(struct >net_device >> fcb = (struct rxfcb *)skb->data; >> >> /* Remove the FCB from the skb */ >> - skb_set_queue_mapping(skb, fcb->rq); >> /* Remove the padded bytes, if there are any */ >> - if (amount_pull) >> + if (amount_pull) { >> + skb_set_queue_mapping(skb, fcb->rq); >> skb_pull(skb, amount_pull); >> + } >> >> if (priv->rx_csum_enable) >> gfar_rx_checksum(skb, fcb); >> >> I only applied the second one and tested again. Right after >the reboot >> I got an error so apparently this change IS required as well. > >I tested this now in the opposite direction and apparently >ONLY this patch is needed. I commented out the second diff and >did not get any errors so far. Sorry for not testing this >before sending my previous mail. > Ok. Thanks ! >Kind regards, >Michael > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c index e0620d0..4577d90 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -2470,10 +2470,11 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb, fcb = (struct rxfcb *)skb->data; /* Remove the FCB from the skb */ - skb_set_queue_mapping(skb, fcb->rq); /* Remove the padded bytes, if there are any */ - if (amount_pull) + if (amount_pull) { + skb_set_queue_mapping(skb, fcb->rq); skb_pull(skb, amount_pull); + } if (priv->rx_csum_enable)