Robert Love wrote:
> On Tue, 2010-05-18 at 10:32 -0700, Joe Eykholt wrote:
>> Joe Eykholt wrote:
>>> Robert Love wrote:
>>>> On Fri, 2010-05-07 at 16:26 -0700, Joe Eykholt wrote:
>>>>> The following series implements the proposed-to-become-standard
>>>>> FIP point-to-multipoint support.
>>>>>
>>>>> The T11 FC-BB-6 committee is proposing a new FIP usage model called
>>>>> VN_port to VN_port mode. It allows VN_ports to discover each other
>>>>> over a loss-free L2 Ethernet without any FCF or Fibre-channel fabric
>>>>> services. This is point-to-multipoint. There is also a mode
>>>>> of this called point-to-point (not implemented here) which provides
>>>>> for making sure there is just one pair of ports operating over
>>>>> the Ethernet fabric.
>>>>>
>>>>> The point-to-multipoint mode will work over direct links and
>>>>> eventually, DCB switches. Cisco switches require turning off
>>>>> the FCoE feature to allow this, however. One shouldn't take
>>>>> this as an indication that switches will or will not more fully
>>>>> support VN2VN mode.
>>>>>
>>>>> See the spec at http://www.t11.org/ftp/t11/pub/fc/bb-6/10-019v2.pdf
>>>>> for more details. That's close, but I know there are updates coming.
>>>>>
>>>>> This part of the BB-6 proposals may become more solid after the
>>>>> next T11 meeting in June or the one after that. The BB-6 standards
>>>>> as a whole may not be ratified until April 2011 or so.
>>>>> I'd be interested to hear about whether these are useful enough
>>>>> to integrate before the standard is completely done.
>>>>>
>>>>> Some of these patches are obviously applicable without the VN2VN
>>>>> changes.
>>>>>
>>>> Hey Joe,
>>>>
>>>> Can you help me understand where the fip->ctlr_src_addr gets updated
>>>> after we have claimed a FCID? As far as I can tell we're setting it in
>>>> fcoe_interface_setup and fcoe_interface_setup, but not anywhere around
>>>> the time we call update_mac in the fcoe_ctlr_vn_timeout work thread.
>>> ctl_src_addr never gets updated, it's always the native MAC address of the
>>> interface. When we get an FC_ID in VN2VN mode, we use update_mac to give
>>> the address to fcoe, which it calls data_src_addr.
>>>
>>>> We're using the fip->ctlr_src_addr to send the CLAIM after
>>>> update_mac,
>>>> but I'm not sure it's being updated.
>>> That's correct. The Claim comes from ctl_src_addr. The contents show the
>>> FC_ID and the MAC we're claiming, not the Ethernet header.
>> To add to this explanation, the MAC descriptor in the claim has the
>> control MAC (what the spec calls ENode MAC), and the Vx_Port_ID descriptor
>> has the FPMA data mac as inserted by this code:
>>
>> frame->vn.fd_desc.fip_dtype = FIP_DT_VN_ID;
>> frame->vn.fd_desc.fip_dlen = sizeof(frame->vn) / FIP_BPW;
>> hton24(frame->vn.fd_mac, FIP_VN_FC_MAP);
>> hton24(frame->vn.fd_mac + 3, fip->port_id);
>> hton24(frame->vn.fd_fc_id, fip->port_id);
>>
>> I'll work on updating the patches to apply on top of the other recent
>> patches.
>>
>
> Thanks for the explanation Joe. I've reviewed all of these patches and
> they look pretty good. Some can probably be consolidated, but I
> appreciate you separating them for review.
>
> I have two quips.
>
> 1) I'm a bit reluctant to have a 4th rport object (the fcoe_vn_port).
> Would it make sense to add some private space at the end of the
> fc_rport_priv so that the allocations aren't separate between libfc and
> libfcoe's rports?
>
> Is there ever a time when you'd have a VN2VN rport, but not a libfc
> rport?
I was also reluctant to add another rport struct, but it seemed necessary.
Even if we combine the allocations, we need the separate structure.
There is a short time where we track the vn_port after receiving a claim
notification from it until we receive a beacon from it where we don't
create the rdata, but we could create the rdata and just not log it in
to eliminate that gap.
There's a locking/lookup issue that makes it better to allocate and
look it up separately, although it may be possible to change fc_rport_priv
to behave appropriately. The issue is that when sending an ELS that needs
to be FIP encapsulated, we need to lookup the vnport to find the correct
destination MAC address. The ELS send is often called while holding the
lport and rport locks, and sometimes the disc_mutex as well, so the
FIP code can't lock the rport list. I made the vnport lookup use
an RCU read lock to handle that.
If we make the fc_rport_lookup() callers hold either the READ read lock
or the disc_mutex, that would be OK. With just the RCU read lock,
they couldn't do a mutex_enter on the rport, but could do a
kref_get(), then drop the RCU lock and then do the mutex_enter(),
but there's no need to change existing callers if they just continue
to hold the disc_mutex, which prevents changes to the list.
It makes some of the rport code a little more tricky.
Overall I don't think there's much to save by changing the rport
lookup to use RCU and have extra space for the vnport structure.
It would save some space, though that wouldn't matter until we
get to big VN2VN configurations. None of this is in the fast path,
and it doesn't save much code. What it saves in FIP, it adds to libfc.
I'm willing to do it, though, if you and others think it's a good idea.
> 2) I think the fcoe_vn_port name might lead to some confusion. It sounds
> like it's for NPIV. Also, it doesn't indicate that it's associated with
> rports, i.e. that it's a "remote" entity. What about something like
> fcoe_vn_rport or fcoe_vnvn_rport? (fcoe_vnvn_rport doesn't match your
> naming convention though)
The specs call all FCoE N_ports VN_Ports. I don't like the term much
either. How about just fcoe_rport and the local variable frport. Sound OK?
This is less specific which might be good if we combine the rport info
later or use it for another type of remote port.
I'm also OK with fcoe_vn_rport if you like that better, but would leave
the local variable named vnport in that case.
> BTW: I'm fine with splitting out the libfcoe.c file into multiple files
> if that helps.
Thanks. I think I'll leave it as one piece for now. It's still a bit
smaller than fcoe.c.
> Thanks, //Rob
Regards,
Joe
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
