firewire: net: fix fragmented datagram_size off-by-one
authorStefan Richter <stefanr@s5r6.in-berlin.de>
Sun, 30 Oct 2016 16:32:01 +0000 (17:32 +0100)
committerStefan Richter <stefanr@s5r6.in-berlin.de>
Thu, 3 Nov 2016 13:46:39 +0000 (14:46 +0100)
RFC 2734 defines the datagram_size field in fragment encapsulation
headers thus:

    datagram_size:  The encoded size of the entire IP datagram.  The
    value of datagram_size [...] SHALL be one less than the value of
    Total Length in the datagram's IP header (see STD 5, RFC 791).

Accordingly, the eth1394 driver of Linux 2.6.36 and older set and got
this field with a -/+1 offset:

    ether1394_tx() /* transmit */
        ether1394_encapsulate_prep()
            hdr->ff.dg_size = dg_size - 1;

    ether1394_data_handler() /* receive */
        if (hdr->common.lf == ETH1394_HDR_LF_FF)
            dg_size = hdr->ff.dg_size + 1;
        else
            dg_size = hdr->sf.dg_size + 1;

Likewise, I observe OS X 10.4 and Windows XP Pro SP3 to transmit 1500
byte sized datagrams in fragments with datagram_size=1499 if link
fragmentation is required.

Only firewire-net sets and gets datagram_size without this offset.  The
result is lacking interoperability of firewire-net with OS X, Windows
XP, and presumably Linux' eth1394.  (I did not test with the latter.)
For example, FTP data transfers to a Linux firewire-net box with max_rec
smaller than the 1500 bytes MTU
  - from OS X fail entirely,
  - from Win XP start out with a bunch of fragmented datagrams which
    time out, then continue with unfragmented datagrams because Win XP
    temporarily reduces the MTU to 576 bytes.

So let's fix firewire-net's datagram_size accessors.

Note that firewire-net thereby loses interoperability with unpatched
firewire-net, but only if link fragmentation is employed.  (This happens
with large broadcast datagrams, and with large datagrams on several
FireWire CardBus cards with smaller max_rec than equivalent PCI cards,
and it can be worked around by setting a small enough MTU.)

Cc: stable@vger.kernel.org
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
drivers/firewire/net.c

index cbaa8d776d83505c1d298b061970b1c44ef10456..15475892af0c948c12f0f5c9ba6375c1f51bdcbf 100644 (file)
@@ -73,13 +73,13 @@ struct rfc2734_header {
 
 #define fwnet_get_hdr_lf(h)            (((h)->w0 & 0xc0000000) >> 30)
 #define fwnet_get_hdr_ether_type(h)    (((h)->w0 & 0x0000ffff))
-#define fwnet_get_hdr_dg_size(h)       (((h)->w0 & 0x0fff0000) >> 16)
+#define fwnet_get_hdr_dg_size(h)       ((((h)->w0 & 0x0fff0000) >> 16) + 1)
 #define fwnet_get_hdr_fg_off(h)                (((h)->w0 & 0x00000fff))
 #define fwnet_get_hdr_dgl(h)           (((h)->w1 & 0xffff0000) >> 16)
 
-#define fwnet_set_hdr_lf(lf)           ((lf)  << 30)
+#define fwnet_set_hdr_lf(lf)           ((lf) << 30)
 #define fwnet_set_hdr_ether_type(et)   (et)
-#define fwnet_set_hdr_dg_size(dgs)     ((dgs) << 16)
+#define fwnet_set_hdr_dg_size(dgs)     (((dgs) - 1) << 16)
 #define fwnet_set_hdr_fg_off(fgo)      (fgo)
 
 #define fwnet_set_hdr_dgl(dgl)         ((dgl) << 16)
@@ -622,7 +622,7 @@ static int fwnet_incoming_packet(struct fwnet_device *dev, __be32 *buf, int len,
                fg_off = fwnet_get_hdr_fg_off(&hdr);
        }
        datagram_label = fwnet_get_hdr_dgl(&hdr);
-       dg_size = fwnet_get_hdr_dg_size(&hdr); /* ??? + 1 */
+       dg_size = fwnet_get_hdr_dg_size(&hdr);
 
        if (fg_off + len > dg_size)
                return 0;