Re: -git tree build failure #2: drivers/net/cnic.c:2520: error: implicit declaration of function ‘__symbol_get’

Re: -git tree build failure #2: drivers/net/cnic.c:2520: error: implicit declaration of function ‘__symbol_get’

[James Bottomley]

On Sat, 2009-06-13 at 17:43 -0700, Michael Chan wrote:
> On Sat, 2009-06-13 at 13:42 -0700, James Bottomley wrote:
> > On Sat, 2009-06-13 at 13:11 -0700, Linus Torvalds wrote:
> > > 
> > > 
> > > That makes no sense. 
> > > 
> > > Look at the first #include in the file - it already includes 
> > > <linux/module.h>.
> > > 
> > > Why do we need to do it twice?
> > 
> > We don't ... it's the wrong fix.  The actual problem is that
> > __symbol_get() is only defined for the modular case.  What it looks to
> > be doing is a reflection call on bnx2_cnic_probe().  I'm not sure why
> > it's doing this ... other than perhaps cnic wants to avoid an explicit
> > bnx2 dependency?  I actually think it's incorrect, since the netdev code
> > before it just checked bnx2 is present, so I see no harm in an explicit
> > call, so this should fix it.
> > 
> > If it had a good reason for the reflective call, then symbol_get()
> > without the __ should be used.
> > 
> > Michael Chan, could you confirm?
> > 
> Thanks James and Ingo.  We don't want to have a symbol dependency on
> bnx2 because this driver eventually will support the 10G bnx2x driver as
> well.  So we want the driver to support either or both NIC drivers
> without both drivers loaded.  Please use the patch below.

Um, but that's not going to work very well.  When you have your 10G
driver, they'll both have to export the symbol name bnx2_cnic_probe
which the kernel isn't going to like.  You can differentiate the symbols
and add a multiple symbol lookup in init_bnx2_cnic(), but that's getting
ugly.

What about doing something more standard, like bus matching?  That's how
the SCSI upper layer drivers work:  we export a virtual SCSI bus and
they bind to it if a supporting device appears.  You could do something
similar exporting a virtual cnic bus from your network drivers and get
the cnic driver to bind to it.

James

Re: -git tree build failure #2: drivers/net/cnic.c:2520: error: implicit declaration of function ‘__symbol_get’

[Michael Chan]

On Sat, 2009-06-13 at 18:33 -0700, James Bottomley wrote:
> On Sat, 2009-06-13 at 17:43 -0700, Michael Chan wrote:
> > On Sat, 2009-06-13 at 13:42 -0700, James Bottomley wrote:
> > > On Sat, 2009-06-13 at 13:11 -0700, Linus Torvalds wrote:
> > > > 
> > > > 
> > > > That makes no sense. 
> > > > 
> > > > Look at the first #include in the file - it already includes 
> > > > <linux/module.h>.
> > > > 
> > > > Why do we need to do it twice?
> > > 
> > > We don't ... it's the wrong fix.  The actual problem is that
> > > __symbol_get() is only defined for the modular case.  What it looks to
> > > be doing is a reflection call on bnx2_cnic_probe().  I'm not sure why
> > > it's doing this ... other than perhaps cnic wants to avoid an explicit
> > > bnx2 dependency?  I actually think it's incorrect, since the netdev code
> > > before it just checked bnx2 is present, so I see no harm in an explicit
> > > call, so this should fix it.
> > > 
> > > If it had a good reason for the reflective call, then symbol_get()
> > > without the __ should be used.
> > > 
> > > Michael Chan, could you confirm?
> > > 
> > Thanks James and Ingo.  We don't want to have a symbol dependency on
> > bnx2 because this driver eventually will support the 10G bnx2x driver as
> > well.  So we want the driver to support either or both NIC drivers
> > without both drivers loaded.  Please use the patch below.
> 
> Um, but that's not going to work very well.  When you have your 10G
> driver, they'll both have to export the symbol name bnx2_cnic_probe
> which the kernel isn't going to like.  You can differentiate the symbols
> and add a multiple symbol lookup in init_bnx2_cnic(), but that's getting
> ugly.

Yeah, the plan is to have a bnx2x_cnic_probe() when we add support for
that.  There will be a separate init_bnx2x_cnic() because the hardware
interface is not exactly the same.

> 
> What about doing something more standard, like bus matching?  That's how
> the SCSI upper layer drivers work:  we export a virtual SCSI bus and
> they bind to it if a supporting device appears.  You could do something
> similar exporting a virtual cnic bus from your network drivers and get
> the cnic driver to bind to it.
> 

This will require some additional infra-structure.  We can look into
this when we support the 10G driver.  Thanks.

Re: -git tree build failure #2: drivers/net/cnic.c:2520: error: implicit declaration of function ‘__symbol_get’

[James Bottomley]

On Sat, 2009-06-13 at 19:18 -0700, Michael Chan wrote:
> On Sat, 2009-06-13 at 18:33 -0700, James Bottomley wrote:
> > On Sat, 2009-06-13 at 17:43 -0700, Michael Chan wrote:
[...]
> > > Thanks James and Ingo.  We don't want to have a symbol dependency on
> > > bnx2 because this driver eventually will support the 10G bnx2x driver as
> > > well.  So we want the driver to support either or both NIC drivers
> > > without both drivers loaded.  Please use the patch below.
> > 
> > Um, but that's not going to work very well.  When you have your 10G
> > driver, they'll both have to export the symbol name bnx2_cnic_probe
> > which the kernel isn't going to like.  You can differentiate the symbols
> > and add a multiple symbol lookup in init_bnx2_cnic(), but that's getting
> > ugly.
> 
> Yeah, the plan is to have a bnx2x_cnic_probe() when we add support for
> that.  There will be a separate init_bnx2x_cnic() because the hardware
> interface is not exactly the same.
> 
> > 
> > What about doing something more standard, like bus matching?  That's how
> > the SCSI upper layer drivers work:  we export a virtual SCSI bus and
> > they bind to it if a supporting device appears.  You could do something
> > similar exporting a virtual cnic bus from your network drivers and get
> > the cnic driver to bind to it.
> > 
> 
> This will require some additional infra-structure.  We can look into
> this when we support the 10G driver.  Thanks.

So if you're going to redo it for the 10G driver, and without the 10G
driver there really seems no need to have a call by reflection in there,
why not simply do a direct call until the 10G driver is added?  This
will save everyone potential hassle from what is a highly unusual
interface.

James

Re: -git tree build failure #2: drivers/net/cnic.c:2520: error: implicit declaration of function '__symbol_get'

[Michael Chan]

James Bottomley wrote:

> On Sat, 2009-06-13 at 19:18 -0700, Michael Chan wrote:
> >
> > This will require some additional infra-structure.  We can look into
> > this when we support the 10G driver.  Thanks.
>
> So if you're going to redo it for the 10G driver, and without the 10G
> driver there really seems no need to have a call by
> reflection in there,
> why not simply do a direct call until the 10G driver is added?  This
> will save everyone potential hassle from what is a highly unusual
> interface.
>

There's an additional usage issue if we switch to a direct call.  The
cnic driver will now have a permanent reference on the bnx2 driver
after it is loaded.  Networking users will be unable to unload the
bnx2 driver once the cnic driver is loaded.

This may not be a serious issue, but one of the goals of the design is
to have minmal impact on networking users.  Since kernel 2.6, users
have been able to unload networking drivers at any time.

Thanks.

Re: -git tree build failure #2: drivers/net/cnic.c:2520: error: implicit declaration of function ‘__symbol_get’

[Mike Christie]

James Bottomley wrote:
> On Sat, 2009-06-13 at 17:43 -0700, Michael Chan wrote:
>> On Sat, 2009-06-13 at 13:42 -0700, James Bottomley wrote:
>>> On Sat, 2009-06-13 at 13:11 -0700, Linus Torvalds wrote:
>>>>
>>>> That makes no sense. 
>>>>
>>>> Look at the first #include in the file - it already includes 
>>>> <linux/module.h>.
>>>>
>>>> Why do we need to do it twice?
>>> We don't ... it's the wrong fix.  The actual problem is that
>>> __symbol_get() is only defined for the modular case.  What it looks to
>>> be doing is a reflection call on bnx2_cnic_probe().  I'm not sure why
>>> it's doing this ... other than perhaps cnic wants to avoid an explicit
>>> bnx2 dependency?  I actually think it's incorrect, since the netdev code
>>> before it just checked bnx2 is present, so I see no harm in an explicit
>>> call, so this should fix it.
>>>
>>> If it had a good reason for the reflective call, then symbol_get()
>>> without the __ should be used.
>>>
>>> Michael Chan, could you confirm?
>>>
>> Thanks James and Ingo.  We don't want to have a symbol dependency on
>> bnx2 because this driver eventually will support the 10G bnx2x driver as
>> well.  So we want the driver to support either or both NIC drivers
>> without both drivers loaded.  Please use the patch below.
> 
> Um, but that's not going to work very well.  When you have your 10G
> driver, they'll both have to export the symbol name bnx2_cnic_probe
> which the kernel isn't going to like.  You can differentiate the symbols
> and add a multiple symbol lookup in init_bnx2_cnic(), but that's getting
> ugly.
> 
> What about doing something more standard, like bus matching?  That's how
> the SCSI upper layer drivers work:  we export a virtual SCSI bus and
> they bind to it if a supporting device appears.  You could do something
> similar exporting a virtual cnic bus from your network drivers and get
> the cnic driver to bind to it.
> 

Something like bus matching would be nice. I think this is going to be a 
bigger problem in the future with everyone putting as many functions on 
a card as possible. We already have the cxgb3 net driver with a iwarp 
(iw_cxgb3) and iscsi (cxgb3i) driver, so maybe something in the net or 
driver model code would be best?

Today, you can't have two pci_drivers attaching to the same device can you?