Re: [PATCH] net/8021q/vlan_dev.c - Use print_mac

2008-02-18 Thread Philip Craig
Joe Perches wrote:
 Perhaps it's more sensible to go back to
 
 #ifdef DEBUG
 #define pr_debug(fmt, arg...) do {} while (0)
 #endif
 
 and give up the printf argument verification

I think argument verification is important.   Can you keep it
like this:

#ifdef DEBUG
#define pr_debug(fmt, arg...) \
do { \
if (0) \
printk(KERN_DEBUG fmt, ##arg); \
} while (0)
#endif

We still lose the return value though, I'm not sure how to keep that
safely in macros.

But does anything rely on the side effects already?  This would
introduce bugs if so.

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/2] Interface groups

2007-10-16 Thread Philip Craig
Laszlo Attila Toth wrote:
 Hello,
 
 Different network interfaces can be grouped using the same group ID. With this
 patch fewer netfilter rules are necessary but it may also be used by routing.

This allows an interface to belong to only one group.  I expect there are
situations where you want more.  eg you might want a group of all pptp
connections, and another group of pptp connections for a subset of users.

An alternative approach would be to extend ipset to have sets of ifindex,
although this would tie it closer to iptables, and it would be slower.
But it still gives the properties of reducing the number of iptables rules,
and allowing to change group membership without reinstalling rules.

Maybe Jozsef has designed nfset to be able to handle this already?
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: bug in tcp?

2007-04-17 Thread Philip Craig
Sebastian Kuzminsky wrote:
 Weird.  Why does sending a message from the client make it go again?

The rule that allows packets with an ESTABLISHED state only matches
packets for which the connection is in netfilter's conntrack table.
The connection is removed from the table after the 5 days of idle.
It is only added again if netfilter accepts a packet for that connection.
So the packet from the client will cause it to be added.

 If that's the case, it seems like a simple fix would be to enable TCP
 keepalive in my app, that would keep netfilter from timing out, right?
 That seems better than extending the netfilter timeout.

Yes that would work.

 How do people normally handle this?

Change the timeout or use keepalives.  I can't think of any other way.
The 5 days is a compromise between keeping valid connections and
timing out dead connections.  There will always be connections for
which it times out too fast or too slow.  I don't think there are
any drawbacks to increasing the timeout if you aren't a router,
but as long as there is a timeout, you need keepalives to be sure.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: bug in tcp?

2007-04-17 Thread Philip Craig
Sebastian Kuzminsky wrote:
 Why did the packet from the client cause the connection to be added back
 to the conntrack table, but the packet from the server did not?

Because the packet from the client was accepted (by a different
iptables rule).

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: bug in tcp?

2007-04-16 Thread Philip Craig
Sebastian Kuzminsky wrote:
 I'm seeing some weird behavior in TCP.  The issue is perfectly
 reproducible using netcat and other programs.  This is what I do:
 
 1.  Open a TCP connection over the loopback (over IPv4).
 
 2.  Send a couple of bytes of data each way.  No problems.
 
 3.  Wait about 120 hours with no writes on either side of the
 connection.
 
 4.  write() a few bytes to the server's socket.  I'd expect the data
 to go through, but it doesnt.  I see the TCP frame from the
 server to the client, but instead of an ACK, the client sends
 back a RST.  netstat shows the bytes sitting in the server's
 socket's send-buffer.
 
 5.  write a few bytes to the client's socket.  The server gets
 these immediately.
 
 6.  On the next server-to-client retransmit, the client gets the
 bytes from the server.  After this, the connection works normally.
 
 
 The libpcap capture file is here (only shows steps 4-6):
 
 http://highlab.com/~seb/tcp-idleness-bug
 
 
 The behavior is reproducible on all kernels I've tried: 2.4.32, 2.6.19.1,
 and 2.6.20.4.  I dont think it's iptables-related, though I'm rerunning
 the tests on a machine without iptables to be sure.  I'll have results
 for you in 120 hours.  ;-)

It sounds like it could easily be iptables related, if you have iptables
rules that only allow new connections in the client to server direction,
which is quite normal.

The default iptables timeout for TCP connections is 5 days.
So after 5 days of idle, any packets from the server will be treated
as a new connection and the iptables rules will drop them.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] r8169: more alignment for the 0x8168

2007-02-25 Thread Philip Craig
Francois Romieu wrote:
 The experimental r8169 patch of the day against 2.6.21-rc1 is available at:
 http://www.fr.zoreil.com/linux/kernel/2.6.x/2.6.21-rc1/

Is 0006-r8169-confusion-between-hardware-and-IP-header-alignment.txt
the only relevant patch?

This only partially helps.  Many of the packets are greater than 200
bytes so copybreak doesn't apply to them.

Can we assume anything about the alignment of skb-data?  I think it
should be 4 byte aligned, otherwise the whole NET_IP_ALIGN thing
won't work.  All the drivers I looked at just reserve NET_IP_ALIGN
without checking the alignment first.

So can you do something like set align to 0 for RTL_CFG_0 and change
rtl8169_alloc_rx_skb() to:
skb_reserve(skb, align ? (align - 1)  (u32)skb-data : NET_IP_ALIGN);

BTW, should the alignment expression be:
(((u32)skb-data + (align - 1))  ~(align - 1)) - (u32)skb-data
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] r8169: more alignment for the 0x8168

2007-02-13 Thread Philip Craig
Francois Romieu wrote:
 Philip Craig [EMAIL PROTECTED] :
 [...]
 This patch caused a drop in throughput from 178 Mbits/sec to 135 Mbits/sec
 on an Intel XScale IXP465.
 
 Which distribution of packet sizes ?

Just using iperf with the default options, MTU 1500,
forwarding between 2 PCs with the board in the middle.

 It seems like there is some confusion about what the align parameter
 here means. It was originally an offset from an aligned address so that
 the IP header aligned, and this patch changes it to the alignment of the
 ethernet header. But align is still set to NET_IP_ALIGN for some chips.
 
 Yes, I should have distinguished both in the first place.
 
 Can you describe which chipset from realtek the board is using (lspci -vxx) ?

# lspci -vxx
00:0c.0 Ethernet controller: Realtek Semiconductor Co., Ltd.: Unknown device 
8169 (rev 10)
Subsystem: Realtek Semiconductor Co., Ltd.: Unknown device 8169
Flags: bus master, 66Mhz, medium devsel, latency 0, IRQ 26
I/O ports at 1000 [size=256]
Memory at 48061000 (32-bit, non-prefetchable) [size=256]
Expansion ROM at 4800 [disabled] [size=128K]
Capabilities: [dc] Power Management version 2
00: ec 10 69 81 57 01 b0 02 10 00 00 02 08 00 00 00
10: 01 10 00 00 00 10 06 48 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 ec 10 69 81
30: 00 00 00 00 dc 00 00 00 00 00 00 00 1a 01 20 40

00:0d.0 Ethernet controller: Realtek Semiconductor Co., Ltd.: Unknown device 
8169 (rev 10)
Subsystem: Realtek Semiconductor Co., Ltd.: Unknown device 8169
Flags: bus master, 66Mhz, medium devsel, latency 0, IRQ 26
I/O ports at 1400 [size=256]
Memory at 48061100 (32-bit, non-prefetchable) [size=256]
Expansion ROM at 4802 [disabled] [size=128K]
Capabilities: [dc] Power Management version 2
00: ec 10 69 81 57 01 b0 02 10 00 00 02 08 00 00 00
10: 01 14 00 00 00 11 06 48 00 00 00 00 00 00 00 00
20: 00 00 00 00 00 00 00 00 00 00 00 00 ec 10 69 81
30: 00 00 00 00 dc 00 00 00 00 00 00 00 1a 01 20 40

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] r8169: more alignment for the 0x8168

2007-02-12 Thread Philip Craig
Francois Romieu wrote:
 Two thirds of packets are lost because of misalignment. Users of
 Asus laptop did apparently not notice it.
 
 Reported on Gigabyte GA-945GM-S2.
 
 Fix for http://bugzilla.kernel.org/show_bug.cgi?id=7517
 
 Signed-off-by: Francois Romieu [EMAIL PROTECTED]
 ---
  drivers/net/r8169.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
 index 0b57050..2379d83 100644
 --- a/drivers/net/r8169.c
 +++ b/drivers/net/r8169.c
 @@ -2018,7 +2018,7 @@ static int rtl8169_alloc_rx_skb(struct p
   if (!skb)
   goto err_out;
  
 - skb_reserve(skb, align);
 + skb_reserve(skb, (align - 1)  (u32)skb-data);
   *sk_buff = skb;
  
   mapping = pci_map_single(pdev, skb-data, rx_buf_sz,
 @@ -2486,7 +2486,7 @@ static inline int rtl8169_try_rx_copy(st
  
   skb = dev_alloc_skb(pkt_size + align);
   if (skb) {
 - skb_reserve(skb, align);
 + skb_reserve(skb, (align - 1)  (u32)skb-data);
   eth_copy_and_sum(skb, sk_buff[0]-data, pkt_size, 0);
   *sk_buff = skb;
   rtl8169_mark_to_asic(desc, rx_buf_sz);

This patch caused a drop in throughput from 178 Mbits/sec to 135 Mbits/sec
on an Intel XScale IXP465.

It seems like there is some confusion about what the align parameter
here means. It was originally an offset from an aligned address so that
the IP header aligned, and this patch changes it to the alignment of the
ethernet header. But align is still set to NET_IP_ALIGN for some chips.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] r8169: more magic during initialization of the hardware

2006-12-05 Thread Philip Craig
Francois Romieu wrote:
 Mostly taken from Realtek's driver.
 
 It's a bit yucky but the original is even worse.

This patch fixes a performance regression for my 8169s.
But it appears to have a typo, see below.


 
 Signed-off-by: Francois Romieu [EMAIL PROTECTED]
 Signed-off-by: Darren Salt [EMAIL PROTECTED]
 ---
  drivers/net/r8169.c |   58 
 +++
  1 files changed, 44 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
 index 45d3ca4..c8fa9b1 100644
 --- a/drivers/net/r8169.c
 +++ b/drivers/net/r8169.c
 @@ -1815,12 +1815,25 @@ static void rtl8169_hw_reset(void __iome
   RTL_R8(ChipCmd);
  }
  
 -static void
 -rtl8169_hw_start(struct net_device *dev)
 +static void rtl8169_set_rx_tx_config_registers(struct rtl8169_private *tp)
 +{
 + void __iomem *ioaddr = tp-mmio_addr;
 + u32 cfg = rtl8169_rx_config;
 +
 + cfg |= (RTL_R32(RxConfig)  rtl_chip_info[tp-chipset].RxConfigMask);
 + RTL_W32(RxConfig, cfg);
 +
 + /* Set DMA burst size and Interframe Gap Time */
 + RTL_W32(TxConfig, (TX_DMA_BURST  TxDMAShift) |
 + (InterFrameGap  TxInterFrameGapShift));
 +}
 +
 +static void rtl8169_hw_start(struct net_device *dev)
  {
   struct rtl8169_private *tp = netdev_priv(dev);
   void __iomem *ioaddr = tp-mmio_addr;
   struct pci_dev *pdev = tp-pci_dev;
 + u16 cmd;
   u32 i;
  
   /* Soft reset the chip. */
 @@ -1833,6 +1846,11 @@ rtl8169_hw_start(struct net_device *dev)
   msleep_interruptible(1);
   }
  
 + if (tp-mac_version == RTL_GIGA_MAC_VER_05) {
 + RTL_W16(CPlusCmd, RTL_R16(CPlusCmd) | PCIMulRW);
 + pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, 0x08);
 + }
 +
   if (tp-mac_version == RTL_GIGA_MAC_VER_13) {
   pci_write_config_word(pdev, 0x68, 0x00);
   pci_write_config_word(pdev, 0x69, 0x08);
 @@ -1840,8 +1858,6 @@ rtl8169_hw_start(struct net_device *dev)
  
   /* Undocumented stuff. */
   if (tp-mac_version == RTL_GIGA_MAC_VER_05) {
 - u16 cmd;
 -
   /* Realtek's r1000_n.c driver uses ' 0x01' here. Well... */
   if ((RTL_R8(Config2)  0x07)  0x01)
   RTL_W32(0x7c, 0x0007);
 @@ -1853,23 +1869,29 @@ rtl8169_hw_start(struct net_device *dev)
   pci_write_config_word(pdev, PCI_COMMAND, cmd);
   }
  
 -
   RTL_W8(Cfg9346, Cfg9346_Unlock);
 + if ((tp-mac_version == RTL_GIGA_MAC_VER_01) ||
 + (tp-mac_version == RTL_GIGA_MAC_VER_02) ||
 + (tp-mac_version == RTL_GIGA_MAC_VER_03) ||
 + (tp-mac_version == RTL_GIGA_MAC_VER_04))
 + RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
 +
   RTL_W8(EarlyTxThres, EarlyTxThld);
  
   /* Low hurts. Let's disable the filtering. */
   RTL_W16(RxMaxSize, 16383);
  
 - /* Set Rx Config register */
 - i = rtl8169_rx_config |
 - (RTL_R32(RxConfig)  rtl_chip_info[tp-chipset].RxConfigMask);
 - RTL_W32(RxConfig, i);
 + if ((tp-mac_version == RTL_GIGA_MAC_VER_01) ||
 + (tp-mac_version == RTL_GIGA_MAC_VER_02) ||
 + (tp-mac_version == RTL_GIGA_MAC_VER_03) ||
 + (tp-mac_version == RTL_GIGA_MAC_VER_04))
 + RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
 + rtl8169_set_rx_tx_config_registers(tp);

Should this RTL_W8() be deleted?
Otherwise there is an indentation/braces mismatch.


  
 - /* Set DMA burst size and Interframe Gap Time */
 - RTL_W32(TxConfig, (TX_DMA_BURST  TxDMAShift) |
 - (InterFrameGap  TxInterFrameGapShift));
 + cmd = RTL_R16(CPlusCmd);
 + RTL_W16(CPlusCmd, cmd);
  
 - tp-cp_cmd |= RTL_R16(CPlusCmd) | PCIMulRW;
 + tp-cp_cmd |= cmd | PCIMulRW;
  
   if ((tp-mac_version == RTL_GIGA_MAC_VER_02) ||
   (tp-mac_version == RTL_GIGA_MAC_VER_03)) {
 @@ -1895,7 +1917,15 @@ rtl8169_hw_start(struct net_device *dev)
   RTL_W32(TxDescStartAddrLow, ((u64) tp-TxPhyAddr  DMA_32BIT_MASK));
   RTL_W32(RxDescAddrHigh, ((u64) tp-RxPhyAddr  32));
   RTL_W32(RxDescAddrLow, ((u64) tp-RxPhyAddr  DMA_32BIT_MASK));
 - RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
 +
 + if ((tp-mac_version != RTL_GIGA_MAC_VER_01) 
 + (tp-mac_version != RTL_GIGA_MAC_VER_02) 
 + (tp-mac_version != RTL_GIGA_MAC_VER_03) 
 + (tp-mac_version != RTL_GIGA_MAC_VER_04)) {
 + RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
 + rtl8169_set_rx_tx_config_registers(tp);
 + }
 +
   RTL_W8(Cfg9346, Cfg9346_Lock);
  
   /* Initially a 10 us delay. Turned it into a PCI commit. - FR */

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] neighbour.c, pneigh_get_next() skips published entry

2006-09-26 Thread Philip Craig
David Miller wrote:
 I've been sitting on this patch because afaik the problem which it purports
 to fix remains unfixed.

 Should I drop it??

 Thanks.
 
 Please drop it, the patch submitted didn't give us the feedback
 and test results we asked for which is necessary to pinpoint the
 true issue here.

It is faster to reproduce with a smaller block size.
I haven't looked in detail to find the cause, but I did notice that
neigh_seq_start() does a pos_minus_one adjustment, and neigh_seq_next()
does not.


First, with only one entry, both block sizes work:

# dd if=/proc/net/arp bs=1024
IP address   HW type Flags   HW addressMask Device
10.46.1.10x1 0x2 00:04:23:C8:8D:E9 *eth0
0+1 records in
0+1 records out
156 bytes (156 B) copied, 0.000161 seconds, 969 kB/s
# dd if=/proc/net/arp bs=128
IP address   HW type Flags   HW addressMask Device
10.46.1.10x1 0x2 00:04:23:C8:8D:E9 *eth0
1+1 records in
1+1 records out
156 bytes (156 B) copied, 0.000193 seconds, 808 kB/s


But add another entry, and it is lost with bs=128:

# arp -Ds 1.1.1.1 eth1 pub
# dd if=/proc/net/arp bs=1024
IP address   HW type Flags   HW addressMask Device
10.46.1.10x1 0x2 00:04:23:C8:8D:E9 *eth0
1.1.1.1  0x1 0xc 00:00:00:00:00:00 *eth1
0+1 records in
0+1 records out
233 bytes (233 B) copied, 2.6e-05 seconds, 9.0 MB/s
# dd if=/proc/net/arp bs=128
IP address   HW type Flags   HW addressMask Device
10.46.1.10x1 0x2 00:04:23:C8:8D:E9 *eth0
1+1 records in
1+1 records out
156 bytes (156 B) copied, 7.9e-05 seconds, 2.0 MB/s


Add more entries, but still only one is lost:

# arp -Ds 1.1.1.2 eth1 pub
# arp -Ds 1.1.1.3 eth1 pub
# arp -Ds 1.1.1.4 eth1 pub
# arp -Ds 1.1.1.5 eth1 pub
# dd if=/proc/net/arp bs=128
IP address   HW type Flags   HW addressMask Device
10.46.1.10x1 0x2 00:04:23:C8:8D:E9 *eth0
1.1.1.3  0x1 0xc 00:00:00:00:00:00 *eth1
1.1.1.2  0x1 0xc 00:00:00:00:00:00 *eth1
1.1.1.5  0x1 0xc 00:00:00:00:00:00 *eth1
1.1.1.4  0x1 0xc 00:00:00:00:00:00 *eth1
3+1 records in
3+1 records out
464 bytes (464 B) copied, 3.6e-05 seconds, 12.9 MB/s
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-14 Thread Philip Craig
Patrick McHardy wrote:
 Joerg Roedel wrote:
 On Thu, Sep 14, 2006 at 11:21:22AM +1000, Philip Craig wrote:

 Joerg Roedel wrote:

 +   To configure tunnels an extra tool is required. You can download
 +   it from http://zlug.fh-zwickau.de/~joro/projects/ under the
 +   EtherIP section. If unsure, say N.
 To obtain a list of tunnels, this tool calls SIOCGETTUNNEL
 (SIOCDEVPRIVATE + 0) for every device in /proc/net/dev. I don't think
 this is safe, but I don't have a solution for you.

 You are right. But this is the way the ipip driver does it. In the case
 of ipip it is safe, because it is visible as a tunnel interface to
 userspace. But my driver registers its devices as Ethernet (it has to,
 otherwise the devices will not be usable in a bridge). There is no safe
 way to distinguish between real Ethernet devices and devices registered
 by my driver. I think about implementing an ioctl to fetch a list of
 all EtherIP tunnel devices from the driver.
 
 
 Just do what ipip and gre do, use a network device with a fixed name
 for the ioctl (you already have the ethip0 device for this purpose it
 appears).

That fixed name device isn't used to get a list of tunnels. Instead,
ipip and gre read /proc/net/dev, and check for ARPHRD_TUNNEL or
ARPHRD_IPGRE. This won't work for etherip because it uses ARPHRD_ETHER,
which isn't specific to etherip tunnels. A new ioctl to get a list could
be added (this ioctl would use the fixed name device), is that acceptable?
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] EtherIP tunnel driver (RFC 3378)

2006-09-13 Thread Philip Craig
Joerg Roedel wrote:
 +  To configure tunnels an extra tool is required. You can download
 +  it from http://zlug.fh-zwickau.de/~joro/projects/ under the
 +  EtherIP section. If unsure, say N.

To obtain a list of tunnels, this tool calls SIOCGETTUNNEL
(SIOCDEVPRIVATE + 0) for every device in /proc/net/dev. I don't think
this is safe, but I don't have a solution for you.

Is there a reason why you have a separate tool rather than modifying
iproute2?

I don't know if you are aware of this older etherip patch by Lennert
Buytenhek: http://www.wantstofly.org/~buytenh/etherip/
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] gre: transparent ethernet bridging

2006-08-06 Thread Philip Craig
Lennert Buytenhek wrote:
 I have one machine at home that appears to be on my employer's network
 via such a tunnel.  I don't use bridging, because I don't need any other
 machine at home to access this tunnel.  I do want bridging, and not proxy
 ARP, because it allows me to run arpwatch, and doesn't require me to
 reconfigure something at the remote end if I, for example, want to add
 another IP address to my home box.

Okay.
If this is using Linux, do you have a patch that does this already?

 No, just send it to the device that matches the lower-level protocol.
 So if you receive an ethernet packet, make it be received on the ethernet
 sub-device, if it's IP, make it be received on the IP sub-device, etc.
 
 Note that I'm not saying that this is necessarily a good idea, it's just
 what I would suggest if you want to run both 'bare IP' and ethernet
 encapsulated traffic over the same GRE tunnel at the same time.  (Not sure
 why you'd want that.)

I still don't understand what the IP sub-device is, or whether
this is different from the options I list below.
But I don't have a good reason for wanting this either.
So lets forget about this and talk about the three options below.

 This seems cleaner to me:
 The GRE tunnel receives a packet of any protocol (ethernet included),
 and adds the IP/GRE header.  Anything else is done above GRE.
 Whatever needs the ethernet header should add it.

 And then the decision is, how do we add the ethernet header?
 - an option to GRE to always add the ethernet header
 
 I think I lost you here.  You surely can't just 'make up' any ethernet
 header?

No, you need a device that looks like an ethernet device.
We can modify the GRE device to look like ethernet (as an option).
Or we can create a new ethernet device stacked on top of the GRE device.
Or we can use the bridge device, which already looks like ethernet.

My first patch in this thread did it by modifying the GRE device.
Look at that patch if my explanation still doesn't make sense.
The option part is controlled by the #if 0 in that patch
(which needs fixing of course).

You have a valid use that doesn't need bridging, so IMO we can rule
out doing it in the bridging code.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] gre: transparent ethernet bridging

2006-08-03 Thread Philip Craig
Lennert Buytenhek wrote:
 So now you _need_ bridging in the middle to send ethernet traffic over
 a GRE tunnel?  Ugh.

Agreed that would not be nice.  What is the usage scenario for this?
At least one end of the tunnel will be bridged?

 If you really want to send ethernet and non-ethernet traffic over the
 same tunnel, can't you make multiple devices?

Do you mean make multiple GRE devices, where one has an ethernet mode set?
That would require more extensive changes to GRE than my original patch.
I don't think being able to send both is important enough to justify that.
Better to just use my original patch if that is acceptable.  There are
parts of it I can clean up more, but nothing major.  And it still needs
some userspace modifications.

Or create a new virtual device to sit between gre and bridging that
simply adds the ethernet header?  This is the most modular approach,
but it seems like overkill for such a simple thing.

 If GRE generally transmits BPDUs without ethernet header, the handling
 for that ought to be in GRE, IMHO.

I don't think this is normal behaviour.  At least, not all routers do it.
More likely some routers have done it as an optimisation.  It does appear
to be specific to GRE.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] gre: transparent ethernet bridging

2006-08-03 Thread Philip Craig
Lennert Buytenhek wrote:
 On Thu, Aug 03, 2006 at 07:14:59PM +1000, Philip Craig wrote:
 
 So now you _need_ bridging in the middle to send ethernet traffic over
 a GRE tunnel?  Ugh.
 Agreed that would not be nice.  What is the usage scenario for this?
 At least one end of the tunnel will be bridged?
 
 For example for VPN purposes, I could imagine that you wouldn't want
 to use bridging.

What is the purpose of including the ethernet header if it only exists
on the tunnel?  I have used GRE tunnels for VPNs, but they have always
needed bridging.  If I don't need bridging, then the VPN is only passing
IP traffic and I just send that without GRE at all.

 If you really want to send ethernet and non-ethernet traffic over the
 same tunnel, can't you make multiple devices?
 Do you mean make multiple GRE devices, where one has an ethernet mode set?
 
 For example.  If you want to send ethernet-encapsulated and other-protocol-
 encapsulated traffic over the same GRE tunnel, that would seem like the
 cleanest solution to me.

It is fine for sending, but when receiving, which packets go to which
device?  Does a packet appear on only one device when sending, but both
devices when receiving?  Or does the operation of the non-ethernet device
change based on whether an ethernet device exists?  Modifying GRE to handle
multiple GRE tunnels that are actually the same does not seem clean.

This seems cleaner to me:
The GRE tunnel receives a packet of any protocol (ethernet included),
and adds the IP/GRE header.  Anything else is done above GRE.
Whatever needs the ethernet header should add it.

And then the decision is, how do we add the ethernet header?
- an option to GRE to always add the ethernet header
- a new virtual device
- as part of bridging

Any of these will work for me.

 Hmm, so it depends on the device whether BPDUs are sent with or without
 an ethernet header?  Do the devices that send BPDUs without ethernet
 header at least accept a BPDU with an ethernet header (and vice versa),
 or aren't the two modes compatible at all?

I don't have any to test with currently, but we only had complaints about
us not receiving them, and only at some sites.  So I assume that devices
can accept either format.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] gre: transparent ethernet bridging

2006-08-02 Thread Philip Craig
 Stephen Hemminger wrote:
 I am not against making the bridge code smarter to handle other
 encapsulation.

Here's an updated patch that fixes all issues I am aware of.

It generates a random mac address for gre ports, and also stores
a copy of the mac address for ethernet ports, rather than checking
dev-type everywhere.

The LLC_SAP_BSPAN packets are handled by simply registering that
protocol with dev_add_pack().  This would have worked for my original
patch too.

I had to release __fake_rtable as part of br_nf_dev_queue_xmit(),
otherwise ip_gre.c paniced trying to call skb-dst-ops-update_ptmu.


--- linux-2.6.x/net/bridge/br.c 18 Jun 2006 23:30:55 -  1.1.1.17
+++ linux-2.6.x/net/bridge/br.c 2 Aug 2006 06:05:10 -
@@ -26,6 +26,11 @@

 int (*br_should_route_hook) (struct sk_buff **pskb) = NULL;

+static struct packet_type br_stp_packet_type = {
+   .type = __constant_htons(LLC_SAP_BSPAN),
+   .func = br_stp_packet_rcv,
+};
+
 static struct llc_sap *br_stp_sap;

 static int __init br_init(void)
@@ -36,6 +41,8 @@ static int __init br_init(void)
return -EBUSY;
}

+   dev_add_pack(br_stp_packet_type);
+   
br_fdb_init();

 #ifdef CONFIG_BRIDGE_NETFILTER
@@ -56,6 +63,7 @@ static int __init br_init(void)
 static void __exit br_deinit(void)
 {
rcu_assign_pointer(br_stp_sap-rcv_func, NULL);
+   dev_remove_pack(br_stp_packet_type);

 #ifdef CONFIG_BRIDGE_NETFILTER
br_netfilter_fini();
--- linux-2.6.x/net/bridge/br_device.c  18 Jun 2006 23:30:55 -  1.1.1.14
+++ linux-2.6.x/net/bridge/br_device.c  2 Aug 2006 06:05:10 -
@@ -95,7 +95,7 @@ static int br_set_mac_address(struct net

spin_lock_bh(br-lock);
list_for_each_entry(port, br-port_list, list) {
-   if (!compare_ether_addr(port-dev-dev_addr, addr-sa_data)) {
+   if (!compare_ether_addr(port-addr.addr, addr-sa_data)) {
br_stp_change_bridge_id(br, addr-sa_data);
err = 0;
break;
--- linux-2.6.x/net/bridge/br_fdb.c 18 Jun 2006 23:30:55 -  1.1.1.13
+++ linux-2.6.x/net/bridge/br_fdb.c 2 Aug 2006 06:05:10 -
@@ -24,8 +24,7 @@
 #include br_private.h

 static kmem_cache_t *br_fdb_cache __read_mostly;
-static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr);
+static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source);

 void __init br_fdb_init(void)
 {
@@ -67,7 +66,7 @@ static __inline__ void fdb_delete(struct
br_fdb_put(f);
 }

-void br_fdb_changeaddr(struct net_bridge_port *p, const unsigned char *newaddr)
+void br_fdb_changeaddr(struct net_bridge_port *p)
 {
struct net_bridge *br = p-br;
int i;
@@ -86,7 +85,7 @@ void br_fdb_changeaddr(struct net_bridge
struct net_bridge_port *op;
list_for_each_entry(op, br-port_list, list) {
if (op != p 
-   
!compare_ether_addr(op-dev-dev_addr,
+   !compare_ether_addr(op-addr.addr,
f-addr.addr)) {
f-dst = op;
goto insert;
@@ -101,7 +100,7 @@ void br_fdb_changeaddr(struct net_bridge
}
  insert:
/* insert new address,  may fail if invalid address or dup. */
-   fdb_insert(br, p, newaddr);
+   fdb_insert(br, p);

spin_unlock_bh(br-hash_lock);
 }
@@ -151,7 +150,7 @@ void br_fdb_delete_by_port(struct net_br
struct net_bridge_port *op;
list_for_each_entry(op, br-port_list, list) {
if (op != p 
-   
!compare_ether_addr(op-dev-dev_addr,
+   !compare_ether_addr(op-addr.addr,
f-addr.addr)) {
f-dst = op;
goto skip_delete;
@@ -291,9 +290,9 @@ static struct net_bridge_fdb_entry *fdb_
return fdb;
 }

-static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr)
+static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source)
 {
+   const unsigned char *addr = source-addr.addr;
struct hlist_head *head = br-hash[br_mac_hash(addr)];
struct net_bridge_fdb_entry *fdb;

@@ -320,13 +319,12 @@ static int fdb_insert(struct net_bridge
return 0;
 }

-int br_fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
- const unsigned char *addr)
+int br_fdb_insert(struct net_bridge *br, struct 

Re: [RFC] gre: transparent ethernet bridging

2006-08-02 Thread Philip Craig
Stephen Hemminger wrote:
 On Wed, 02 Aug 2006 16:17:42 +1000
 Philip Craig [EMAIL PROTECTED] wrote:
 It generates a random mac address for gre ports, and also stores
 a copy of the mac address for ethernet ports, rather than checking
 dev-type everywhere.

 That looks cleaner. I wonder if using a fixed OUI would be better
 than random addresses but then choosing an OUI would be a problem.

random_ether_addr() sets the local assignment bit. This is what
various other virtual devices do (including tap devices, which can
also be bridged).

 You probably should add a comment about what this function is doing,
 and why.

Okay.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] gre: transparent ethernet bridging

2006-08-01 Thread Philip Craig
Stephen Hemminger wrote:
 I am not against making the bridge code smarter to handle other
 encapsulation.

Do you mean something like this patch?

The only drawback I see for this approach is that it means you
can only encapsulate the ethernet header if the gre interface is
bridged.  That's not too bad a restriction though.

This patch only works for local packets so far, and doesn't
handle the LLC_SAP_BSPAN packets.

Also, if the gre interface is the only port on the bridge, then
we have no mac address.


--- linux-2.6.x/net/bridge/br_device.c  18 Jun 2006 23:30:55 -  1.1.1.14
+++ linux-2.6.x/net/bridge/br_device.c  1 Aug 2006 09:12:42 -
@@ -17,6 +17,7 @@
 #include linux/netdevice.h
 #include linux/etherdevice.h
 #include linux/ethtool.h
+#include linux/if_arp.h

 #include asm/uaccess.h
 #include br_private.h
@@ -95,7 +96,9 @@ static int br_set_mac_address(struct net

spin_lock_bh(br-lock);
list_for_each_entry(port, br-port_list, list) {
-   if (!compare_ether_addr(port-dev-dev_addr, addr-sa_data)) {
+   if (port-dev-type == ARPHRD_ETHER 
+   !compare_ether_addr(port-dev-dev_addr,
+   addr-sa_data)) {
br_stp_change_bridge_id(br, addr-sa_data);
err = 0;
break;
--- linux-2.6.x/net/bridge/br_fdb.c 18 Jun 2006 23:30:55 -  1.1.1.13
+++ linux-2.6.x/net/bridge/br_fdb.c 1 Aug 2006 09:12:42 -
@@ -20,6 +20,7 @@
 #include linux/netdevice.h
 #include linux/etherdevice.h
 #include linux/jhash.h
+#include linux/if_arp.h
 #include asm/atomic.h
 #include br_private.h

@@ -86,6 +87,7 @@ void br_fdb_changeaddr(struct net_bridge
struct net_bridge_port *op;
list_for_each_entry(op, br-port_list, list) {
if (op != p 
+   op-dev-type == ARPHRD_ETHER 

!compare_ether_addr(op-dev-dev_addr,
f-addr.addr)) {
f-dst = op;
@@ -151,6 +153,7 @@ void br_fdb_delete_by_port(struct net_br
struct net_bridge_port *op;
list_for_each_entry(op, br-port_list, list) {
if (op != p 
+   op-dev-type == ARPHRD_ETHER 

!compare_ether_addr(op-dev-dev_addr,
f-addr.addr)) {
f-dst = op;
--- linux-2.6.x/net/bridge/br_forward.c 18 Jun 2006 23:30:55 -  1.1.1.15
+++ linux-2.6.x/net/bridge/br_forward.c 1 Aug 2006 09:12:42 -
@@ -18,6 +18,7 @@
 #include linux/skbuff.h
 #include linux/if_vlan.h
 #include linux/netfilter_bridge.h
+#include linux/if_arp.h
 #include br_private.h

 static inline int should_deliver(const struct net_bridge_port *p,
@@ -46,6 +47,8 @@ int br_dev_queue_push_xmit(struct sk_buf
nf_bridge_maybe_copy_header(skb);
 #endif
skb_push(skb, ETH_HLEN);
+   if (skb-dev-type == ARPHRD_IPGRE)
+   skb-protocol = htons(ETH_P_BRIDGE);

dev_queue_xmit(skb);
}
--- linux-2.6.x/net/bridge/br_if.c  18 Jun 2006 23:30:55 -  1.1.1.23
+++ linux-2.6.x/net/bridge/br_if.c  1 Aug 2006 09:12:42 -
@@ -391,7 +391,10 @@ int br_add_if(struct net_bridge *br, str
struct net_bridge_port *p;
int err = 0;

-   if (dev-flags  IFF_LOOPBACK || dev-type != ARPHRD_ETHER)
+   if (dev-flags  IFF_LOOPBACK)
+   return -EINVAL;
+
+   if (dev-type != ARPHRD_ETHER  dev-type != ARPHRD_IPGRE)
return -EINVAL;

if (dev-hard_start_xmit == br_dev_xmit)
@@ -408,9 +411,11 @@ int br_add_if(struct net_bridge *br, str
if (err)
goto err0;

-   err = br_fdb_insert(br, p, dev-dev_addr);
-   if (err)
-   goto err1;
+   if (dev-type == ARPHRD_ETHER) {
+   err = br_fdb_insert(br, p, dev-dev_addr);
+   if (err)
+   goto err1;
+   }

err = br_sysfs_addif(p);
if (err)
--- linux-2.6.x/net/bridge/br_input.c   18 Jun 2006 23:30:55 -  1.1.1.18
+++ linux-2.6.x/net/bridge/br_input.c   1 Aug 2006 09:12:42 -
@@ -17,6 +17,7 @@
 #include linux/netdevice.h
 #include linux/etherdevice.h
 #include linux/netfilter_bridge.h
+#include linux/if_arp.h
 #include br_private.h

 /* Bridge group multicast address 802.1d (pg 51). */
@@ -124,11 +125,22 @@ static inline int is_link_local(const un
 int br_handle_frame(struct net_bridge_port *p, struct sk_buff **pskb)
 {
struct sk_buff *skb = *pskb;
-   const unsigned 

[RFC] gre: transparent ethernet bridging

2006-07-31 Thread Philip Craig
This patch implements transparent ethernet bridging for gre tunnels.
There are a few outstanding issues.

There is no way for userspace to select the type of gre tunnel. The
#if 0 near the top of the patch forces all gre tunnels to be bridges.
The problem is that userspace uses an IPPROTO_ to select the type of
tunnel, but both types of gre tunnel are IPPROTO_GRE. I can't see
anything else in struct ip_tunnel_parm that could be used to select
this. One approach that I've seen mentioned in the archives is to add
a netlink interface to replace the tunnel ioctls.

Network loops are bad. See the comments at the top of ip_gre.c for
a description of how gre tunnels handle this normally. But for gre
bridges, we don't want to copy the ttl (it breaks routing protocols),
and we don't want to force DF (we want to bridge 1500 byte packets).
I couldn't think of any solution for this.

Some routers set LLC_SAP_BSPAN in the gre protocol field, and then
give the bpdu packet without any other ethernet/llc header. This patch
currently tries to fake the ethernet/llc header before passing the
packet up, but it is buggy (mac addresses are wrong at least). Maybe a
better approach is to call directly into the bridging code. I didn't try
that at first because it isn't modular, and may break other things that
want to see the packet.


--- linux-2.6.x/net/ipv4/ip_gre.c   18 Jun 2006 23:30:56 -  1.1.1.33
+++ linux-2.6.x/net/ipv4/ip_gre.c   31 Jul 2006 09:57:41 -
@@ -30,6 +30,8 @@
 #include linux/igmp.h
 #include linux/netfilter_ipv4.h
 #include linux/if_ether.h
+#include linux/etherdevice.h
+#include linux/llc.h

 #include net/sock.h
 #include net/ip.h
@@ -41,6 +43,8 @@
 #include net/dsfield.h
 #include net/inet_ecn.h
 #include net/xfrm.h
+#include net/llc.h
+#include net/llc_pdu.h

 #ifdef CONFIG_IPV6
 #include net/ipv6.h
@@ -119,6 +123,7 @@

 static int ipgre_tunnel_init(struct net_device *dev);
 static void ipgre_tunnel_setup(struct net_device *dev);
+static void ipgre_ether_tunnel_setup(struct net_device *dev);

 /* Fallback tunnel: no source, no destination, no key, no options */

@@ -274,7 +279,11 @@ static struct ip_tunnel * ipgre_tunnel_l
goto failed;
}

+#if 0
dev = alloc_netdev(sizeof(*t), name, ipgre_tunnel_setup);
+#else
+   dev = alloc_netdev(sizeof(*t), name, ipgre_ether_tunnel_setup);
+#endif
if (!dev)
  return NULL;

@@ -550,6 +559,68 @@ ipgre_ecn_encapsulate(u8 tos, struct iph
return INET_ECN_encapsulate(tos, inner);
 }

+__be16 ipgre_type_trans(struct sk_buff *skb, int offset)
+{
+   u8 *h = skb-data;
+   __be16 flags = *(__be16*)h;
+   __be16 proto = *(__be16*)(h + 2);
+
+   /* WCCP version 1 and 2 protocol decoding.
+* - Change protocol to IP
+* - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
+*/
+   if (flags == 0 
+   proto == __constant_htons(ETH_P_WCCP)) {
+   proto = __constant_htons(ETH_P_IP);
+   if ((*(h + offset)  0xF0) != 0x40)
+   offset += 4;
+   }
+
+   skb-mac.raw = skb-nh.raw;
+   skb-nh.raw = __pskb_pull(skb, offset);
+   skb_postpull_rcsum(skb, skb-h.raw, offset);
+#ifdef CONFIG_NET_IPGRE_BROADCAST
+   if (MULTICAST(iph-daddr)) {
+   /* Looped back packet, drop it! */
+   if (((struct rtable*)skb-dst)-fl.iif == 0)
+   return 0;
+   /* tunnel-stat.multicast++; */
+   skb-pkt_type = PACKET_BROADCAST;
+   }
+#endif
+
+   return proto;
+}
+
+extern const u8 br_group_address[ETH_ALEN];
+
+__be16 ipgre_ether_type_trans(struct sk_buff *skb, struct net_device *dev,
+ int offset)
+{
+   u8 *h = skb-data;
+   __be16 proto = *(__be16*)(h + 2);
+
+   if (proto == htons(ETH_P_BRIDGE)) {
+   if (!pskb_may_pull(skb, offset + ETH_HLEN))
+   return 0;
+   skb_pull_rcsum(skb, offset);
+   return eth_type_trans(skb, dev);
+   } else if (proto == htons(LLC_SAP_BSPAN)) {
+   skb_pull_rcsum(skb, offset);
+
+   llc_pdu_header_init(skb, LLC_PDU_TYPE_U, LLC_SAP_BSPAN,
+   LLC_SAP_BSPAN, LLC_PDU_CMD);
+   llc_pdu_init_as_ui_cmd(skb);
+
+   llc_mac_hdr_init(skb, dev-dev_addr, dev-dev_addr);
+   skb_pull(skb, ETH_HLEN);
+
+   return htons(ETH_P_802_2);
+   }
+
+   return 0;
+}
+
 static int ipgre_rcv(struct sk_buff *skb)
 {
struct iphdr *iph;
@@ -603,32 +674,8 @@ static int ipgre_rcv(struct sk_buff *skb
if ((tunnel = ipgre_tunnel_lookup(iph-saddr, iph-daddr, key)) != 
NULL) {
secpath_reset(skb);

-   skb-protocol = *(u16*)(h + 2);
-   /* WCCP version 1 and 2 protocol decoding.
-* - Change protocol to IP
-* - When dealing with WCCPv2, 

Re: [RFC] gre: transparent ethernet bridging

2006-07-31 Thread Philip Craig
Stephen Hemminger wrote:
 On Mon, 31 Jul 2006 20:06:41 +1000
 Philip Craig [EMAIL PROTECTED] wrote:
 
 This patch implements transparent ethernet bridging for gre tunnels.
 There are a few outstanding issues.
 
 Why not use existing bridge code?

It does use the existing bridge code.  Perhaps the name is misleading.
All it does is encapsulate the full ethernet header in a gre packet,
rather than only layer 3.  That is, currently gre uses ARPHRD_IPGRE,
but bridging requires ARPHRD_ETHER.

 Some routers set LLC_SAP_BSPAN in the gre protocol field, and then
 give the bpdu packet without any other ethernet/llc header. This patch
 currently tries to fake the ethernet/llc header before passing the
 packet up, but it is buggy (mac addresses are wrong at least). Maybe a
 better approach is to call directly into the bridging code. I didn't try
 that at first because it isn't modular, and may break other things that
 want to see the packet.
 
 Existing bridge code already has spanning tree.

Yes, and I want to use that.  But this packet is a bit strange in
that it does not have the ethernet header on it.   So what is the
best way to pass it to existing code?  Either fake the ethernet
header, or pass it directly?

 +#if 0
  dev = alloc_netdev(sizeof(*t), name, ipgre_tunnel_setup);
 +#else
 +dev = alloc_netdev(sizeof(*t), name, ipgre_ether_tunnel_setup);
 +#endif
 
 Do, or do not there is no try

I am looking for comments as to whether adding a netlink interface
to control this is appropriate.

 +__be16 ipgre_type_trans(struct sk_buff *skb, int offset)
 +{
 +u8 *h = skb-data;
 +__be16 flags = *(__be16*)h;
 +__be16 proto = *(__be16*)(h + 2);
 +
 +/* WCCP version 1 and 2 protocol decoding.
 + * - Change protocol to IP
 + * - When dealing with WCCPv2, Skip extra 4 bytes in GRE header
 + */
 +if (flags == 0 
 +proto == __constant_htons(ETH_P_WCCP)) {
 +proto = __constant_htons(ETH_P_IP);
 +if ((*(h + offset)  0xF0) != 0x40)
 +offset += 4;
 +}
 
 Don't use __constant_htons() except in initializers and switch cases
 (where gcc is too stupid to optimize the macro).
 

This is a problem in the existing code, which I am simply moving
around.  Should I fix it at the same time?
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: New Qlogic qla3xxx NIC Driver v2.02.00k31 for upstream inclusion

2006-06-22 Thread Philip Craig
On 06/22/2006 02:55 PM, Andrew Morton wrote:
 - Is there a better way of doing this?

 static void ql_swap_mac_addr(u8 * macAddress)
 {
 #ifdef __BIG_ENDIAN
   u8 temp;
   temp = macAddress[0];
   macAddress[0] = macAddress[1];
   macAddress[1] = temp;
   temp = macAddress[2];
   macAddress[2] = macAddress[3];
   macAddress[3] = temp;
   temp = macAddress[4];
   macAddress[4] = macAddress[5];
   macAddress[5] = temp;
 #endif
 }

Perhaps something like:

static void ql_swap_mac_addr(u8 * macAddress)
{
u16 *p = (u16 *)macAddress;

cpu_to_le16s(p);
cpu_to_le16s(p+1);
cpu_to_le16s(p+2);
}

You could use cpu_to_le16s for the version/numPorts conversion too.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2 resend] 8139cp: add ethtool eeprom support

2006-06-20 Thread Philip Craig
Implement the ethtool eeprom operations for the 8139cp driver.
Tested on x86 and big-endian ARM.

Signed-off-by: Philip Craig [EMAIL PROTECTED]

Index: linux-2.6.17-rc6/drivers/net/8139cp.c
===
--- linux-2.6.17-rc6.orig/drivers/net/8139cp.c  2006-06-14 15:59:26.0 
+1000
+++ linux-2.6.17-rc6/drivers/net/8139cp.c   2006-06-14 15:59:53.0 
+1000
@@ -401,6 +401,11 @@ static void cp_clean_rings (struct cp_pr
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void cp_poll_controller(struct net_device *dev);
 #endif
+static int cp_get_eeprom_len(struct net_device *dev);
+static int cp_get_eeprom(struct net_device *dev,
+struct ethtool_eeprom *eeprom, u8 *data);
+static int cp_set_eeprom(struct net_device *dev,
+struct ethtool_eeprom *eeprom, u8 *data);
 
 static struct pci_device_id cp_pci_tbl[] = {
{ PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139,
@@ -1577,6 +1582,9 @@ static struct ethtool_ops cp_ethtool_ops
.get_strings= cp_get_strings,
.get_ethtool_stats  = cp_get_ethtool_stats,
.get_perm_addr  = ethtool_op_get_perm_addr,
+   .get_eeprom_len = cp_get_eeprom_len,
+   .get_eeprom = cp_get_eeprom,
+   .set_eeprom = cp_set_eeprom,
 };
 
 static int cp_ioctl (struct net_device *dev, struct ifreq *rq, int cmd)
@@ -1612,24 +1620,32 @@ static int cp_ioctl (struct net_device *
 #define eeprom_delay() readl(ee_addr)
 
 /* The EEPROM commands include the alway-set leading bit. */
+#define EE_EXTEND_CMD  (4)
 #define EE_WRITE_CMD   (5)
 #define EE_READ_CMD(6)
 #define EE_ERASE_CMD   (7)
 
-static int read_eeprom (void __iomem *ioaddr, int location, int addr_len)
-{
-   int i;
-   unsigned retval = 0;
-   void __iomem *ee_addr = ioaddr + Cfg9346;
-   int read_cmd = location | (EE_READ_CMD  addr_len);
+#define EE_EWDS_ADDR   (0)
+#define EE_WRAL_ADDR   (1)
+#define EE_ERAL_ADDR   (2)
+#define EE_EWEN_ADDR   (3)
+
+#define CP_EEPROM_MAGIC PCI_DEVICE_ID_REALTEK_8139
 
+static void eeprom_cmd_start(void __iomem *ee_addr)
+{
writeb (EE_ENB  ~EE_CS, ee_addr);
writeb (EE_ENB, ee_addr);
eeprom_delay ();
+}
+
+static void eeprom_cmd(void __iomem *ee_addr, int cmd, int cmd_len)
+{
+   int i;
 
-   /* Shift the read command bits out. */
-   for (i = 3 + addr_len - 1; i = 0; i--) {
-   int dataval = (read_cmd  (1  i)) ? EE_DATA_WRITE : 0;
+   /* Shift the command bits out. */
+   for (i = cmd_len - 1; i = 0; i--) {
+   int dataval = (cmd  (1  i)) ? EE_DATA_WRITE : 0;
writeb (EE_ENB | dataval, ee_addr);
eeprom_delay ();
writeb (EE_ENB | dataval | EE_SHIFT_CLK, ee_addr);
@@ -1637,6 +1653,33 @@ static int read_eeprom (void __iomem *io
}
writeb (EE_ENB, ee_addr);
eeprom_delay ();
+}
+
+static void eeprom_cmd_end(void __iomem *ee_addr)
+{
+   writeb (~EE_CS, ee_addr);
+   eeprom_delay ();
+}
+
+static void eeprom_extend_cmd(void __iomem *ee_addr, int extend_cmd,
+ int addr_len)
+{
+   int cmd = (EE_EXTEND_CMD  addr_len) | (extend_cmd  (addr_len - 2));
+
+   eeprom_cmd_start(ee_addr);
+   eeprom_cmd(ee_addr, cmd, 3 + addr_len);
+   eeprom_cmd_end(ee_addr);
+}
+
+static u16 read_eeprom (void __iomem *ioaddr, int location, int addr_len)
+{
+   int i;
+   u16 retval = 0;
+   void __iomem *ee_addr = ioaddr + Cfg9346;
+   int read_cmd = location | (EE_READ_CMD  addr_len);
+
+   eeprom_cmd_start(ee_addr);
+   eeprom_cmd(ee_addr, read_cmd, 3 + addr_len);
 
for (i = 16; i  0; i--) {
writeb (EE_ENB | EE_SHIFT_CLK, ee_addr);
@@ -1648,13 +1691,125 @@ static int read_eeprom (void __iomem *io
eeprom_delay ();
}
 
-   /* Terminate the EEPROM access. */
-   writeb (~EE_CS, ee_addr);
-   eeprom_delay ();
+   eeprom_cmd_end(ee_addr);
 
return retval;
 }
 
+static void write_eeprom(void __iomem *ioaddr, int location, u16 val,
+int addr_len)
+{
+   int i;
+   void __iomem *ee_addr = ioaddr + Cfg9346;
+   int write_cmd = location | (EE_WRITE_CMD  addr_len);
+
+   eeprom_extend_cmd(ee_addr, EE_EWEN_ADDR, addr_len);
+
+   eeprom_cmd_start(ee_addr);
+   eeprom_cmd(ee_addr, write_cmd, 3 + addr_len);
+   eeprom_cmd(ee_addr, val, 16);
+   eeprom_cmd_end(ee_addr);
+
+   eeprom_cmd_start(ee_addr);
+   for (i = 0; i  2; i++)
+   if (readb(ee_addr)  EE_DATA_READ)
+   break;
+   eeprom_cmd_end(ee_addr);
+
+   eeprom_extend_cmd(ee_addr, EE_EWDS_ADDR, addr_len);
+}
+
+static int cp_get_eeprom_len(struct net_device *dev)
+{
+   struct cp_private *cp = netdev_priv(dev);
+   int size;
+
+   spin_lock_irq(cp-lock

[PATCH 1/2 resend] 8139cp: fix eeprom read command length

2006-06-20 Thread Philip Craig
The read command for the 93C46/93C56 EEPROMS should be 3 bits plus
the address.  This doesn't appear to affect the operation of the
read command, but similar errors for write commands do cause failures.

Signed-off-by: Philip Craig [EMAIL PROTECTED]

Index: linux-2.6.17-rc6/drivers/net/8139cp.c
===
--- linux-2.6.17-rc6.orig/drivers/net/8139cp.c  2006-06-14 16:02:00.0 
+1000
+++ linux-2.6.17-rc6/drivers/net/8139cp.c   2006-06-14 16:03:29.0 
+1000
@@ -1628,7 +1628,7 @@ static int read_eeprom (void __iomem *io
eeprom_delay ();
 
/* Shift the read command bits out. */
-   for (i = 4 + addr_len; i = 0; i--) {
+   for (i = 3 + addr_len - 1; i = 0; i--) {
int dataval = (read_cmd  (1  i)) ? EE_DATA_WRITE : 0;
writeb (EE_ENB | dataval, ee_addr);
eeprom_delay ();

--

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] 8139cp: add ethtool eeprom support

2006-06-14 Thread Philip Craig
Implement the ethtool eeprom operations for the 8139cp driver.
Tested on x86 and big-endian ARM.

Signed-off-by: Philip Craig [EMAIL PROTECTED]

Index: linux-2.6.17-rc6/drivers/net/8139cp.c
===
--- linux-2.6.17-rc6.orig/drivers/net/8139cp.c  2006-06-14 15:59:26.0 
+1000
+++ linux-2.6.17-rc6/drivers/net/8139cp.c   2006-06-14 15:59:53.0 
+1000
@@ -401,6 +401,11 @@ static void cp_clean_rings (struct cp_pr
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void cp_poll_controller(struct net_device *dev);
 #endif
+static int cp_get_eeprom_len(struct net_device *dev);
+static int cp_get_eeprom(struct net_device *dev,
+struct ethtool_eeprom *eeprom, u8 *data);
+static int cp_set_eeprom(struct net_device *dev,
+struct ethtool_eeprom *eeprom, u8 *data);

 static struct pci_device_id cp_pci_tbl[] = {
{ PCI_VENDOR_ID_REALTEK, PCI_DEVICE_ID_REALTEK_8139,
@@ -1577,6 +1582,9 @@ static struct ethtool_ops cp_ethtool_ops
.get_strings= cp_get_strings,
.get_ethtool_stats  = cp_get_ethtool_stats,
.get_perm_addr  = ethtool_op_get_perm_addr,
+   .get_eeprom_len = cp_get_eeprom_len,
+   .get_eeprom = cp_get_eeprom,
+   .set_eeprom = cp_set_eeprom,
 };

 static int cp_ioctl (struct net_device *dev, struct ifreq *rq, int cmd)
@@ -1612,24 +1620,32 @@ static int cp_ioctl (struct net_device *
 #define eeprom_delay() readl(ee_addr)

 /* The EEPROM commands include the alway-set leading bit. */
+#define EE_EXTEND_CMD  (4)
 #define EE_WRITE_CMD   (5)
 #define EE_READ_CMD(6)
 #define EE_ERASE_CMD   (7)

-static int read_eeprom (void __iomem *ioaddr, int location, int addr_len)
-{
-   int i;
-   unsigned retval = 0;
-   void __iomem *ee_addr = ioaddr + Cfg9346;
-   int read_cmd = location | (EE_READ_CMD  addr_len);
+#define EE_EWDS_ADDR   (0)
+#define EE_WRAL_ADDR   (1)
+#define EE_ERAL_ADDR   (2)
+#define EE_EWEN_ADDR   (3)
+
+#define CP_EEPROM_MAGIC PCI_DEVICE_ID_REALTEK_8139

+static void eeprom_cmd_start(void __iomem *ee_addr)
+{
writeb (EE_ENB  ~EE_CS, ee_addr);
writeb (EE_ENB, ee_addr);
eeprom_delay ();
+}
+
+static void eeprom_cmd(void __iomem *ee_addr, int cmd, int cmd_len)
+{
+   int i;

-   /* Shift the read command bits out. */
-   for (i = 3 + addr_len - 1; i = 0; i--) {
-   int dataval = (read_cmd  (1  i)) ? EE_DATA_WRITE : 0;
+   /* Shift the command bits out. */
+   for (i = cmd_len - 1; i = 0; i--) {
+   int dataval = (cmd  (1  i)) ? EE_DATA_WRITE : 0;
writeb (EE_ENB | dataval, ee_addr);
eeprom_delay ();
writeb (EE_ENB | dataval | EE_SHIFT_CLK, ee_addr);
@@ -1637,6 +1653,33 @@ static int read_eeprom (void __iomem *io
}
writeb (EE_ENB, ee_addr);
eeprom_delay ();
+}
+
+static void eeprom_cmd_end(void __iomem *ee_addr)
+{
+   writeb (~EE_CS, ee_addr);
+   eeprom_delay ();
+}
+
+static void eeprom_extend_cmd(void __iomem *ee_addr, int extend_cmd,
+ int addr_len)
+{
+   int cmd = (EE_EXTEND_CMD  addr_len) | (extend_cmd  (addr_len - 2));
+
+   eeprom_cmd_start(ee_addr);
+   eeprom_cmd(ee_addr, cmd, 3 + addr_len);
+   eeprom_cmd_end(ee_addr);
+}
+
+static u16 read_eeprom (void __iomem *ioaddr, int location, int addr_len)
+{
+   int i;
+   u16 retval = 0;
+   void __iomem *ee_addr = ioaddr + Cfg9346;
+   int read_cmd = location | (EE_READ_CMD  addr_len);
+
+   eeprom_cmd_start(ee_addr);
+   eeprom_cmd(ee_addr, read_cmd, 3 + addr_len);

for (i = 16; i  0; i--) {
writeb (EE_ENB | EE_SHIFT_CLK, ee_addr);
@@ -1648,13 +1691,125 @@ static int read_eeprom (void __iomem *io
eeprom_delay ();
}

-   /* Terminate the EEPROM access. */
-   writeb (~EE_CS, ee_addr);
-   eeprom_delay ();
+   eeprom_cmd_end(ee_addr);

return retval;
 }

+static void write_eeprom(void __iomem *ioaddr, int location, u16 val,
+int addr_len)
+{
+   int i;
+   void __iomem *ee_addr = ioaddr + Cfg9346;
+   int write_cmd = location | (EE_WRITE_CMD  addr_len);
+
+   eeprom_extend_cmd(ee_addr, EE_EWEN_ADDR, addr_len);
+
+   eeprom_cmd_start(ee_addr);
+   eeprom_cmd(ee_addr, write_cmd, 3 + addr_len);
+   eeprom_cmd(ee_addr, val, 16);
+   eeprom_cmd_end(ee_addr);
+
+   eeprom_cmd_start(ee_addr);
+   for (i = 0; i  2; i++)
+   if (readb(ee_addr)  EE_DATA_READ)
+   break;
+   eeprom_cmd_end(ee_addr);
+
+   eeprom_extend_cmd(ee_addr, EE_EWDS_ADDR, addr_len);
+}
+
+static int cp_get_eeprom_len(struct net_device *dev)
+{
+   struct cp_private *cp = netdev_priv(dev);
+   int size;
+
+   spin_lock_irq(cp-lock);
+   size

[PATCH 1/2] 8139cp: fix eeprom read command length

2006-06-14 Thread Philip Craig
The read command for the 93C46/93C56 EEPROMS should be 3 bits plus
the address.  This doesn't appear to affect the operation of the
read command, but similar errors for write commands do cause failures.

Signed-off-by: Philip Craig [EMAIL PROTECTED]

Index: linux-2.6.17-rc6/drivers/net/8139cp.c
===
--- linux-2.6.17-rc6.orig/drivers/net/8139cp.c  2006-06-14 16:02:00.0 
+1000
+++ linux-2.6.17-rc6/drivers/net/8139cp.c   2006-06-14 16:03:29.0 
+1000
@@ -1628,7 +1628,7 @@ static int read_eeprom (void __iomem *io
eeprom_delay ();

/* Shift the read command bits out. */
-   for (i = 4 + addr_len; i = 0; i--) {
+   for (i = 3 + addr_len - 1; i = 0; i--) {
int dataval = (read_cmd  (1  i)) ? EE_DATA_WRITE : 0;
writeb (EE_ENB | dataval, ee_addr);
eeprom_delay ();
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC: 2.6 patch] net/netlink/: possible cleanups

2006-04-18 Thread Philip Craig
On 04/14/2006 06:26 AM, David S. Miller wrote:
 These interfaces were added so that new users of netlink could
 write their code more easily.

 Unused does not equate to comment out or delete.

Does a GENETLINK Kconfig option make sense (possibly dependant on
EMBEDDED)?  I'm unsure whether these interfaces are going to be used
in core networking code that can't be disabled anyway.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html