On Wed, 2010-09-22 at 14:37 -0700, Joe Eykholt wrote:
>
> On 9/22/10 12:39 PM, Robert Love wrote:
<snip>
> > @@ -1262,16 +1261,7 @@ int fcoe_rcv(struct sk_buff *skb, struct net_device
> > *netdev,
> > skb_tail_pointer(skb), skb_end_pointer(skb),
> > skb->csum, skb->dev ? skb->dev->name : "<NULL>");
> >
> > - /* check for mac addresses */
> > eh = eth_hdr(skb);
> > - port = lport_priv(lport);
> > - if (compare_ether_addr(eh->h_dest, port->data_src_addr) &&
> > - compare_ether_addr(eh->h_dest, fcoe->ctlr.ctl_src_addr) &&
> > - compare_ether_addr(eh->h_dest, (u8[6])FC_FCOE_FLOGI_MAC)) {
> > - FCOE_NETDEV_DBG(netdev, "wrong destination mac address:%pM\n",
> > - eh->h_dest);
> > - goto err;
> > - }
> >
> > if (is_fip_mode(&fcoe->ctlr) &&
> > compare_ether_addr(eh->h_source, fcoe->ctlr.dest_addr)) {
> > @@ -1291,6 +1281,13 @@ int fcoe_rcv(struct sk_buff *skb, struct net_device
> > *netdev,
> > skb_set_transport_header(skb, sizeof(struct fcoe_hdr));
> > fh = (struct fc_frame_header *) skb_transport_header(skb);
> >
> > + if (compare_ether_addr(eh->h_dest, (u8[6])FC_FCOE_FLOGI_MAC) &&
> > + ntoh24(&eh->h_dest[3]) != ntoh24(fh->fh_d_id)) {
> > + FCOE_NETDEV_DBG(netdev, "bad destination MAC:%pM\n",
> > + eh->h_dest);
> > + goto err;
> > + }
>
> This seems OK to me, although it doesn't check the first 3 bytes, which could
> be different for each lport. That seems like an OK compromise. We could
> re-check
> the mac after the lport was determined later, but that would be an
> Ethernet-specific
> check in libfc. An LLD callback. I think what you have is OK.
>
Thanks for the review Joe.
The LLD callback was considered. I really dislike that solution though.
For one it adds another entry to the libfc template, which isn't
terrible, but I'd prefer not to bloat it if possible. More importantly,
I think it's bad to allow the frame past fcoe and into libfc to later
callback to fcoe to double-check the frame. I like the fcoe validation
to be early and complete and then if the frame passes it can be sent up
the stack.
My (never mailed) original patch was checking the upper bytes against
the VN2VN-MAP and the FC-MAP, but I think the HW MAC filtering should
cover those bits. I decided against adding another check since I thought
it was redundant. Also, reading the spec very literally, it only talks
about the "destination MAC
address/destination N_Port_ID" not the "destination MAC
address/destination N_Port_ID AND FC-MAP."
> You can drop the FLOGI_MAC check since its last 3 bytes matches the D_ID used
> for FLOGI.
>
Oh, excellent, will do- and resend.
Thanks, //Rob
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel
