Looking at mac_promisc_remove()...
mac_promisc_remove(mac_promisc_handle_t mph)
{
mac_promisc_impl_t *mpip = (mac_promisc_impl_t *)mph;
mac_client_impl_t *mcip = mpip->mpi_mcip;
mac_impl_t *mip = mcip->mci_mip;
mac_cb_info_t *mcbi;
int rc = 0;
i_mac_perim_enter(mip);
/*
* Even if the device can''t be reset into normal mode, we still
* need to clear the client promisc callbacks. The client may want
* to close the mac end point and we can''t have stale
callbacks.
*/
if (!(mpip->mpi_no_phys)) {
rc = mac_promisc_set((mac_handle_t)mip, B_FALSE,
MAC_DEVPROMISC);
*** if (rc != 0)
*** goto done;
}
mcbi = &mip->mi_promisc_cb_info;
mutex_enter(mcbi->mcbi_lockp);
if (mac_callback_remove(mcbi, &mip->mi_promisc_list,
&mpip->mpi_mi_link)) {
VERIFY(mac_callback_remove(&mip->mi_promisc_cb_info,
&mcip->mci_promisc_list,
&mpip->mpi_mci_link));
kmem_cache_free(mac_promisc_impl_cache, mpip);
} else {
mac_callback_remove_wait(&mip->mi_promisc_cb_info);
}
mutex_exit(mcbi->mcbi_lockp);
mac_stop((mac_handle_t)mip);
***done:
i_mac_perim_exit(mip);
return (rc);
}
It would seem to me that the lines that start with "***" need
to be deleted because they are at odds with the comment, namely
that if the call to mac_promisc_set() fails, the callback isn''t
removed from the list.
Well either the code is wrong or the bug is wrong...
But ideally, as a client, what I''d like to be able to do is:
(void) mac_promisc_remove(mch);
and know that my callbacks have been removed and not need to
worry about any error.
but I don''t know what''s right here: code or comment?
I just know that both can''t be :)
Darren
I think you are referring to old code. The new code has it correct. http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/mac/mac_client.c#2393 ~Girish Darren Reed wrote:> Looking at mac_promisc_remove()... > > mac_promisc_remove(mac_promisc_handle_t mph) > { > mac_promisc_impl_t *mpip = (mac_promisc_impl_t *)mph; > mac_client_impl_t *mcip = mpip->mpi_mcip; > mac_impl_t *mip = mcip->mci_mip; > mac_cb_info_t *mcbi; > int rc = 0; > > i_mac_perim_enter(mip); > > /* > * Even if the device can''t be reset into normal mode, we still > * need to clear the client promisc callbacks. The client may want > * to close the mac end point and we can''t have stale callbacks. > */ > if (!(mpip->mpi_no_phys)) { > rc = mac_promisc_set((mac_handle_t)mip, B_FALSE, > MAC_DEVPROMISC); > *** if (rc != 0) > *** goto done; > } > mcbi = &mip->mi_promisc_cb_info; > mutex_enter(mcbi->mcbi_lockp); > if (mac_callback_remove(mcbi, &mip->mi_promisc_list, > &mpip->mpi_mi_link)) { > VERIFY(mac_callback_remove(&mip->mi_promisc_cb_info, > &mcip->mci_promisc_list, &mpip->mpi_mci_link)); > kmem_cache_free(mac_promisc_impl_cache, mpip); > } else { > mac_callback_remove_wait(&mip->mi_promisc_cb_info); > } > mutex_exit(mcbi->mcbi_lockp); > mac_stop((mac_handle_t)mip); > > ***done: > i_mac_perim_exit(mip); > return (rc); > } > > It would seem to me that the lines that start with "***" need > to be deleted because they are at odds with the comment, namely > that if the call to mac_promisc_set() fails, the callback isn''t > removed from the list. > > Well either the code is wrong or the bug is wrong... > > But ideally, as a client, what I''d like to be able to do is: > > (void) mac_promisc_remove(mch); > > and know that my callbacks have been removed and not need to > worry about any error. > > but I don''t know what''s right here: code or comment? > I just know that both can''t be :) > > Darren > > > _______________________________________________ > crossbow-discuss mailing list > crossbow-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/crossbow-discuss
On 30/03/09 07:27 PM, Girish M G wrote:> I think you are referring to old code. The new code has it correct. > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/mac/mac_client.c#2393thanks... the code I was looking at isn''t *that* old! Darren