netdev - Re: [net-next (TAKE 2) 3/4] firewire net, ipv4 arp: Extend hardware address and remove driver-level packet inspection.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 10 Feb 2013 15:31:07 +0100
From:	Stefan Richter <stefanr@...6.in-berlin.de>
To:	YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org>
Cc:	netdev@...r.kernel.org, linux1394-devel@...ts.sourceforge.net,
	davem@...emloft.net
Subject: Re: [net-next (TAKE 2) 3/4] firewire net, ipv4 arp: Extend hardware
 address and remove driver-level packet inspection.

On Feb 10 YOSHIFUJI Hideaki wrote:
> @@ -1339,42 +1236,23 @@ static netdev_tx_t fwnet_tx(struct sk_buff *skb, struct net_device *net)
>  		ptask->dest_node   = IEEE1394_ALL_NODES;
>  		ptask->speed       = SCODE_100;
>  	} else {
> -		__be64 guid = get_unaligned((__be64 *)hdr_buf.h_dest);
> +		union fwnet_hwaddr *ha = (union fwnet_hwaddr *)hdr_buf.h_dest;
> +		__be64 guid = get_unaligned(&ha->uc.uniq_id);
>  		u8 generation;
>  
>  		peer = fwnet_peer_find_by_guid(dev, be64_to_cpu(guid));
> -		if (!peer || peer->fifo == FWNET_NO_FIFO_ADDR)
> +		if (!peer)
>  			goto fail;
>  
>  		generation         = peer->generation;
>  		dest_node          = peer->node_id;
> -		max_payload        = peer->max_payload;
> +		max_payload        = fwnet_hwaddr_maxpayload(ha);
>  		datagram_label_ptr = &peer->datagram_label;
>  
> -		ptask->fifo_addr   = peer->fifo;
> +		ptask->fifo_addr   = fwnet_hwaddr_fifo(ha);
>  		ptask->generation  = generation;
>  		ptask->dest_node   = dest_node;
> -		ptask->speed       = peer->speed;
> -	}
> -
> -	/* If this is an ARP packet, convert it */
> -	if (proto == htons(ETH_P_ARP)) {
> -		struct arphdr *arp = (struct arphdr *)skb->data;
> -		unsigned char *arp_ptr = (unsigned char *)(arp + 1);
> -		struct rfc2734_arp *arp1394 = (struct rfc2734_arp *)skb->data;
> -		__be32 ipaddr;
> -
> -		ipaddr = get_unaligned((__be32 *)(arp_ptr + FWNET_ALEN));
> -
> -		arp1394->hw_addr_len    = RFC2734_HW_ADDR_LEN;
> -		arp1394->max_rec        = dev->card->max_receive;
> -		arp1394->sspd		= dev->card->link_speed;
> -
> -		put_unaligned_be16(dev->local_fifo >> 32,
> -				   &arp1394->fifo_hi);
> -		put_unaligned_be32(dev->local_fifo & 0xffffffff,
> -				   &arp1394->fifo_lo);
> -		put_unaligned(ipaddr, &arp1394->sip);
> +		ptask->speed       = fwnet_fixup_speed(ha->uc.sspd);
>  	}
>  
>  	ptask->hdr.w0 = 0;
> @@ -1491,13 +1369,9 @@ static int fwnet_add_peer(struct fwnet_device *dev,
>  
>  	peer->dev = dev;
>  	peer->guid = (u64)device->config_rom[3] << 32 | device->config_rom[4];
> -	peer->fifo = FWNET_NO_FIFO_ADDR;
> -	peer->ip = 0;
>  	INIT_LIST_HEAD(&peer->pd_list);
>  	peer->pdg_size = 0;
>  	peer->datagram_label = 0;
> -	peer->speed = device->max_speed;
> -	peer->max_payload = fwnet_max_payload(device->max_rec, peer->speed);
>  
>  	peer->generation = device->generation;
>  	smp_rmb();

As far as I can tell, it would be best to ignore max_rec and sspd from ARP
and NDP but keep using the respective information from firewire-core
instead (handed over by fwnet_probe()).

Why?  As I noted earlier, RFC 2734:1999 and RFC 3146:2001 were apparently
written with a too simplistic notion of IEEE 1394 bus topology, resulting
in max_rec and sspd in ARP-1394 and NDP-1394 to be useless, IMO.

Consider a bus like this:

    A ---- B ==== C

A, B, C are all IP-over-1394 capable nodes.  ---- is an S400 cable hop,
and ==== is an S800 cable hop.

In case of unicasts or multicasts in which node A is involved as
transmitter or receiver, as well as in case of broadcasts, the speeds
S100, S200, S400 work and speed S400 is optimal.

In case of anything else, IOW in case of unicasts or multicasts in which
only nodes B and C are involved, the speeds S100, S200, S400, S800 work
and speed S800 is optimal.

Clearly, node A should indicate sspd = S400 in its ARP or NDP packets.
But which sspd should nodes B and C set there?  Maybe they set S400, which
would work but would waste half of the available bandwidth in the second
case.  Or maybe they set S800, which is OK in the second case but would
prohibit any communication with node A if blindly taken for correct.

On the other hand, firewire-core *always* gives us the correct and optimum
peer-to-peer speed and asynchronous packet payload, no matter how simple
or complex the bus topology is and no matter in which temporal order nodes
join the bus and are discovered.
-- 
Stefan Richter
-=====-===-= --=- -=-=-
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html