Hi! Sreekanth - this does look like it is valid and needs fixing. Just file a FreeBSD PR (bugs.freebsd.org/submit/) and we'll assign it to the intel team.
Thanks! -a On 27 January 2015 at 12:42, Jack Vogel <jfvo...@gmail.com> wrote: > Yes, I will look them over. > > Jack > > > On Tue, Jan 27, 2015 at 12:38 PM, Sreekanth Rupavatharam < > rupav...@juniper.net> wrote: > >> Thanks jack, >> Now, can you please review these changes? And commit if you deem it >> fit? >> >> Thanks, >> >> -Sreekanth >> >> On Jan 27, 2015, at 12:24 PM, "Jack Vogel" <jfvo...@gmail.com> wrote: >> >> Errrr, I am one of those people :) (jack.vo...@intel.com) >> >> Jack >> >> >> On Tue, Jan 27, 2015 at 12:21 PM, Sreekanth Rupavatharam < >> rupav...@juniper.net> wrote: >> >>> Definitely, but I didn't have the contact info of those people. >>> >>> Thanks, >>> >>> -Sreekanth >>> >>> On Jan 27, 2015, at 12:15 PM, "Jack Vogel" <jfvo...@gmail.com> wrote: >>> >>> If you want something committed to an Intel driver, asking Intel might >>> be the >>> courteous thing to do, don't you think? >>> >>> Jack >>> >>> >>> On Tue, Jan 27, 2015 at 11:51 AM, Sreekanth Rupavatharam < >>> rupav...@juniper.net> wrote: >>> >>>> Hiren, >>>> Can you help commit this? >>>> >>>> Index: if_igb.c >>>> >>>> =================================================================== >>>> >>>> --- if_igb.c (revision 298053) >>>> >>>> +++ if_igb.c (working copy) >>>> >>>> @@ -723,7 +723,8 @@ igb_attach(device_t dev) >>>> >>>> return (0); >>>> >>>> >>>> >>>> err_late: >>>> >>>> - igb_detach(dev); >>>> >>>> + if(igb_detach(dev) == 0) /* igb_detach did the cleanup */ >>>> >>>> + return(error); >>>> >>>> igb_free_transmit_structures(adapter); >>>> >>>> igb_free_receive_structures(adapter); >>>> >>>> igb_release_hw_control(adapter); >>>> >>>> -- Thanks, >>>> >>>> Sreekanth >>>> >>>> >>>> >>>> >>>> >>>> >>>> On 1/27/15, 11:28 AM, "hiren panchasara" <hi...@strugglingcoder.info> >>>> wrote: >>>> >>>> + Jack >>>> On Tue, Jan 27, 2015 at 12:00:19AM +0000, Sreekanth Rupavatharam wrote: >>>> >>>> Apologies if this is not the right forum. In igb_attach function, we >>>> have this code. >>>> err_late: >>>> igb_detach(dev); >>>> igb_free_transmit_structures(adapter); >>>> igb_free_receive_structures(adapter); >>>> igb_release_hw_control(adapter); >>>> err_pci: >>>> igb_free_pci_resources(adapter); >>>> if (adapter->ifp != NULL) >>>> if_free(adapter->ifp); >>>> free(adapter->mta, M_DEVBUF); >>>> IGB_CORE_LOCK_DESTROY(adapter); >>>> The problem is that igb_detach also does the same cleanup in it?s >>>> body. Only exception is this case where it just returns EBUSY >>>> /* Make sure VLANS are not using driver */ >>>> if (if_vlantrunkinuse(ifp)) { >>>> device_printf(dev,"Vlan in use, detach first\n"); >>>> return (EBUSY); >>>> } >>>> I think the code in igb_attach should be changed to free up resources >>>> only if the igb_detach returns an error. Here?s the patch for it. >>>> Index: if_igb.c >>>> =================================================================== >>>> --- if_igb.c (revision 298025) >>>> +++ if_igb.c (working copy) >>>> @@ -723,7 +723,8 @@ igb_attach(device_t dev) >>>> return (0); >>>> err_late: >>>> - igb_detach(dev); >>>> + if(igb_detach(dev) == 0) /* igb_detach did the cleanup */ >>>> + return; >>>> igb_free_transmit_structures(adapter); >>>> Can anyone comment on it and tell me if my understanding is >>>> incorrect? >>>> >>>> >>>> Seems reasonable to me at the first glance. >>>> >>>> We need to call IGB_CORE_LOCK_DESTROY(adapter) before returning though. >>>> >>>> cheers, >>>> Hiren >>>> >>>> >>> >> > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org" _______________________________________________ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"