Vasu Dev wrote:
> On Mon, 2009-03-23 at 16:49 -0700, Joe Eykholt wrote:
>> Vasu Dev wrote:
>>> + if (netdev->netdev_ops && netdev->netdev_ops->ndo_get_storage_address) {
>>> + memcpy(fc->ctlr.ctl_src_addr,
>>> + netdev->netdev_ops->ndo_get_storage_address(netdev),
>>> + ETH_ALEN);
>>> + if (is_valid_ether_addr(fc->ctlr.ctl_src_addr))
>>> + fc->ctlr.spma = 1;
>> Wouldn't it be a bug in the LLD to give an invalid MAC here?
>> It's OK to check, though.
>>
>
> Let us leave this in to ensure MAC address validity, however LLD are not
> expected to give bad MAC address, anyway this code is not in fast path
> so one additional check won't be expensive here.
>
>>> sol->fip.fip_dl_len = htons(sizeof(sol->desc) / FIP_BPW);
>>> sol->fip.fip_flags = htons(FIP_FL_FPMA);
>>> + if (fip->spma)
>>> + sol->fip.fip_flags |= htons(FIP_FL_SPMA);
>> Lets's add a u16 capability field in the fcoe_ctlr, which would
>> contain FPMA or SPMA or both, and here we would just use that to set the
>> flags in the packet. We could use a be16 field, but u16 is better so
>> we can test it instead of using fip->spma.
>>
>
> I think using u8 spma flag makes code clean by simply checking a flag,
> otherwise spma bit in u16 or b16 would have required read, bit-wise-AND
> and then check in execution. We are not forcing spma or fpma mode only
> with these patches so adding a explicit bit or non-bit flag for fpma
> might me redundant at this point. If needed then I'll add u16 flag
> later.
>>> - if (dtype != ELS_FLOGI)
>>> - memcpy(mac->fd_mac, fip->data_src_addr, ETH_ALEN);
>>> + if (fip->spma)
>>> + memcpy(mac->fd_mac, fip->ctl_src_addr, ETH_ALEN);
>> For non-FLOGI, non-SPMA, I think we want the data_src address still.
>>
>
> The fd_mac was never set before due to a wrong ELS_FLOGI check and now
> with this change it will be set to only ctl_src_addr, I see this is also
> not good. Good catch, I'll provide fix for this.
No, it was correct before. On FLOGIs we want the MAC descriptor mac to be zero
unless we're using SPMA (that should be based on the selected FCF, not
fip->spma).
For other ELSes, we want the assigned data_src_addr, as it was before.
See section 7.7.7.4 of fip-09-117v1.pdf from fcoe.com or t11.org.
I'll consider the issues from your other mail a bit longer.
Joe
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
