Re: [PATCHv2 2.6.18-rc1-mm2 1/3] net: UDP-Lite generic support

2006-07-16 Thread Gerrit Renker
Quoting Herbert Xu:
|                  case SO_NO_CHECK:
|   -                       sk-sk_no_check = valbool;
|   +                       /* UDP-Lite (RFC 3828) mandates checksumming,
|   +                        * hence user must not enable this option.   */
|   +                       if (sk-sk_protocol == IPPROTO_UDPLITE)
|   +                               ret = -EOPNOTSUPP;
|   +                       else
|   +                           sk-sk_no_check = valbool;
|  
|  Please don't add protocol-specific stuff to generic functions.  In this
|  case why don't you just ignore sk_no_check for UDPLITE as we do for TCP?

Thank you for spotting this -- the UDP-Lite code indeed ignores sk_no_check
and will (if no socket options are set) emulate UDP with sk_no_check = 0. 
Setting
it to 1 will make no difference; so the above is more not strictly necessary. 
Will 
remove in next patch.
Any other comments or ideas, please do not hesitate to write. 

-- Gerrit
-
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] clear skb cb on IP input

2006-07-16 Thread chas williams - CONTRACTOR
In message [EMAIL PROTECTED],David Miller writes:
From: Herbert Xu [EMAIL PROTECTED]
 At least this lets us get rid of a few other memsets :)
 
Applied, good spotting :-)  I remember when we added those
things.

But I'm beginning to think that the onus of this may in fact fall upon
the devices, in fact.  Loopback is one of the few devices where the
control block might not be cleared out, due to uses in the output
path.  Devices predominantly provide a zero'd out control block in the
skb on packet receive.

the atm layer has the same problem.  some atm device drivers use the
skb cb field, so it needs to be zero'ed by the next upper layer (clip,
lane) before being passed to the ip layer.  its also possible that the
atm layer should clone the skb before passing to the next layer which
would also zeroize the cb.

Other opinions welcome...

why does the input side of the ip layer believe the cb contains valid
data?  it should zero the contents of the cb, or just fill in the cb
correctly when the packet arrives at its doorstop.
-
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] WAN: converting generic HDLC to use netif_dormant*()

2006-07-16 Thread Krzysztof Halasa
David Miller [EMAIL PROTECTED] writes:

 WARNING: hdlc_set_carrier [drivers/char/synclinkmp.ko] undefined!
 WARNING: hdlc_set_carrier [drivers/char/synclink_gt.ko] undefined!
 WARNING: hdlc_set_carrier [drivers/char/synclink.ko] undefined!
 WARNING: hdlc_set_carrier [drivers/char/pcmcia/synclink_cs.ko] undefined!

 Krzysztof please provide a fix for this,

Sure

 this function you added
 calls to does not exist anywhere,

The function doesn't exist anymore, I've removed the calls (but
unfortunately not all of them). Now, instead of hdlc_set_carrier(),
normal netif_carrier_on|off() should be used (it looks like only
synclink drivers and drivers written by me used hdlc_set_carrier()
- other drivers always used netif_carrier_*() only).

 which suggests you either produced
 a patch against a different tree than the current one or you didn't
 even compile test this patch.

Actually I wasn't at the moment aware that there are WAN drivers in
drivers/char (only checked drivers/net).

I'm just posting a patch.
-- 
Krzysztof Halasa
-
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] WAN: Added missing netif_dormant_off() to generic HDLC

2006-07-16 Thread Krzysztof Halasa
WAN: Fixed a problem with PPP/raw HDLC/X.25 protocols not doing
netif_dormant_off() at startup.

Signed-off-by: Krzysztof Halasa [EMAIL PROTECTED]

diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index b81263e..fbaab5b 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -107,6 +107,7 @@ int hdlc_ppp_ioctl(struct net_device *de
dev-hard_header = NULL;
dev-type = ARPHRD_PPP;
dev-addr_len = 0;
+   netif_dormant_off(dev);
return 0;
}
 
diff --git a/drivers/net/wan/hdlc_raw.c b/drivers/net/wan/hdlc_raw.c
index 9456d31..f15aa6b 100644
--- a/drivers/net/wan/hdlc_raw.c
+++ b/drivers/net/wan/hdlc_raw.c
@@ -82,6 +82,7 @@ int hdlc_raw_ioctl(struct net_device *de
dev-type = ARPHRD_RAWHDLC;
dev-flags = IFF_POINTOPOINT | IFF_NOARP;
dev-addr_len = 0;
+   netif_dormant_off(dev);
return 0;
}
 
diff --git a/drivers/net/wan/hdlc_raw_eth.c b/drivers/net/wan/hdlc_raw_eth.c
index b1285cc..d188498 100644
--- a/drivers/net/wan/hdlc_raw_eth.c
+++ b/drivers/net/wan/hdlc_raw_eth.c
@@ -100,6 +100,7 @@ int hdlc_raw_eth_ioctl(struct net_device
dev-tx_queue_len = old_qlen;
memcpy(dev-dev_addr, \x00\x01, 2);
 get_random_bytes(dev-dev_addr + 2, ETH_ALEN - 2);
+   netif_dormant_off(dev);
return 0;
}
 
diff --git a/drivers/net/wan/hdlc_x25.c b/drivers/net/wan/hdlc_x25.c
index 07e5eef..a867fb4 100644
--- a/drivers/net/wan/hdlc_x25.c
+++ b/drivers/net/wan/hdlc_x25.c
@@ -212,6 +212,7 @@ int hdlc_x25_ioctl(struct net_device *de
dev-hard_header = NULL;
dev-type = ARPHRD_X25;
dev-addr_len = 0;
+   netif_dormant_off(dev);
return 0;
}
 
-
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] WAN: Cosmetic changes to N2 and C101 drivers

2006-07-16 Thread Krzysztof Halasa
WAN: Cosmetic changes to N2 and C101 drivers

Signed-off-by: Krzysztof Halasa [EMAIL PROTECTED]

diff --git a/drivers/net/wan/c101.c b/drivers/net/wan/c101.c
index 3796a59..2911be3 100644
--- a/drivers/net/wan/c101.c
+++ b/drivers/net/wan/c101.c
@@ -197,7 +197,6 @@ static int c101_open(struct net_device *
sca_out(IE0_TXINT, MSCI0_OFFSET + IE0, port);
 
set_carrier(port);
-   printk(KERN_DEBUG 0x%X\n, sca_in(MSCI1_OFFSET + ST3, port));
 
/* enable MSCI1 CDCD interrupt */
sca_out(IE1_CDCD, MSCI1_OFFSET + IE1, port);
@@ -449,4 +448,5 @@ module_exit(c101_cleanup);
 MODULE_AUTHOR(Krzysztof Halasa [EMAIL PROTECTED]);
 MODULE_DESCRIPTION(Moxa C101 serial port driver);
 MODULE_LICENSE(GPL v2);
-module_param(hw, charp, 0444); /* hw=irq,ram:irq,... */
+module_param(hw, charp, 0444);
+MODULE_PARM_DESC(hw, irq,ram:irq,...);
diff --git a/drivers/net/wan/n2.c b/drivers/net/wan/n2.c
index cd32751..0a17ad6 100644
--- a/drivers/net/wan/n2.c
+++ b/drivers/net/wan/n2.c
@@ -559,4 +559,5 @@ module_exit(n2_cleanup);
 MODULE_AUTHOR(Krzysztof Halasa [EMAIL PROTECTED]);
 MODULE_DESCRIPTION(RISCom/N2 serial port driver);
 MODULE_LICENSE(GPL v2);
-module_param(hw, charp, 0444); /* hw=io,irq,ram,ports:io,irq,... */
+module_param(hw, charp, 0444);
+MODULE_PARM_DESC(hw, io,irq,ram,ports:io,irq,...);
-
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] WAN: Converted synclink drivers to use netif_carrier_*()

2006-07-16 Thread Krzysztof Halasa
WAN: Converted synclink drivers to use netif_carrier_*() instead
of hdlc_set_carrier().

Signed-off-by: Krzysztof Halasa [EMAIL PROTECTED]

diff --git a/drivers/char/pcmcia/synclink_cs.c 
b/drivers/char/pcmcia/synclink_cs.c
index 0721345..bdd09d9 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -1174,8 +1174,12 @@ static void dcd_change(MGSLPC_INFO *info
else
info-input_signal_events.dcd_down++;
 #ifdef CONFIG_HDLC
-   if (info-netcount)
-   hdlc_set_carrier(info-serial_signals  SerialSignal_DCD, 
info-netdev);
+   if (info-netcount) {
+   if (info-serial_signals  SerialSignal_DCD)
+   netif_carrier_on(info-netdev);
+   else
+   netif_carrier_off(info-netdev);
+   }
 #endif
wake_up_interruptible(info-status_event_wait_q);
wake_up_interruptible(info-event_wait_q);
@@ -4251,8 +4255,10 @@ static int hdlcdev_open(struct net_devic
spin_lock_irqsave(info-lock, flags);
get_signals(info);
spin_unlock_irqrestore(info-lock, flags);
-   hdlc_set_carrier(info-serial_signals  SerialSignal_DCD, dev);
-
+   if (info-serial_signals  SerialSignal_DCD)
+   netif_carrier_on(dev);
+   else
+   netif_carrier_off(dev);
return 0;
 }
 
diff --git a/drivers/char/synclink.c b/drivers/char/synclink.c
index fee2aca..ecf4786 100644
--- a/drivers/char/synclink.c
+++ b/drivers/char/synclink.c
@@ -1344,8 +1344,12 @@ static void mgsl_isr_io_pin( struct mgsl
} else
info-input_signal_events.dcd_down++;
 #ifdef CONFIG_HDLC
-   if (info-netcount)
-   hdlc_set_carrier(status  MISCSTATUS_DCD, 
info-netdev);
+   if (info-netcount) {
+   if (status  MISCSTATUS_DCD)
+   netif_carrier_on(info-netdev);
+   else
+   netif_carrier_off(info-netdev);
+   }
 #endif
}
if (status  MISCSTATUS_CTS_LATCHED)
@@ -7844,8 +7848,10 @@ static int hdlcdev_open(struct net_devic
spin_lock_irqsave(info-irq_spinlock, flags);
usc_get_serial_signals(info);
spin_unlock_irqrestore(info-irq_spinlock, flags);
-   hdlc_set_carrier(info-serial_signals  SerialSignal_DCD, dev);
-
+   if (info-serial_signals  SerialSignal_DCD)
+   netif_carrier_on(dev);
+   else
+   netif_carrier_off(dev);
return 0;
 }
 
diff --git a/drivers/char/synclink_gt.c b/drivers/char/synclink_gt.c
index b4d1f4e..03f5cdf 100644
--- a/drivers/char/synclink_gt.c
+++ b/drivers/char/synclink_gt.c
@@ -1497,8 +1497,10 @@ static int hdlcdev_open(struct net_devic
spin_lock_irqsave(info-lock, flags);
get_signals(info);
spin_unlock_irqrestore(info-lock, flags);
-   hdlc_set_carrier(info-signals  SerialSignal_DCD, dev);
-
+   if (info-signals  SerialSignal_DCD)
+   netif_carrier_on(dev);
+   else
+   netif_carrier_off(dev);
return 0;
 }
 
@@ -1997,8 +1999,12 @@ static void dcd_change(struct slgt_info 
info-input_signal_events.dcd_down++;
}
 #ifdef CONFIG_HDLC
-   if (info-netcount)
-   hdlc_set_carrier(info-signals  SerialSignal_DCD, 
info-netdev);
+   if (info-netcount) {
+   if (info-signals  SerialSignal_DCD)
+   netif_carrier_on(info-netdev);
+   else
+   netif_carrier_off(info-netdev);
+   }
 #endif
wake_up_interruptible(info-status_event_wait_q);
wake_up_interruptible(info-event_wait_q);
diff --git a/drivers/char/synclinkmp.c b/drivers/char/synclinkmp.c
index 8587401..9681255 100644
--- a/drivers/char/synclinkmp.c
+++ b/drivers/char/synclinkmp.c
@@ -1753,8 +1753,10 @@ static int hdlcdev_open(struct net_devic
spin_lock_irqsave(info-lock, flags);
get_signals(info);
spin_unlock_irqrestore(info-lock, flags);
-   hdlc_set_carrier(info-serial_signals  SerialSignal_DCD, dev);
-
+   if (info-serial_signals  SerialSignal_DCD)
+   netif_carrier_on(dev);
+   else
+   netif_carrier_off(dev);
return 0;
 }
 
@@ -2523,8 +2525,12 @@ void isr_io_pin( SLMP_INFO *info, u16 st
} else
info-input_signal_events.dcd_down++;
 #ifdef CONFIG_HDLC
-   if (info-netcount)
-   hdlc_set_carrier(status  SerialSignal_DCD, 
info-netdev);
+   if (info-netcount) {
+   if (status  SerialSignal_DCD)
+   netif_carrier_on(info-netdev);
+   else
+   

Re: [2.6 patch] drivers/net/wireless/zd1211rw/: possible cleanups

2006-07-16 Thread Daniel Drake

Adrian Bunk wrote:

This patch contains the following possible cleanups:
- make needlessly global functions static
- #if 0 unused functions

Please review which of these functions do make sense and which do 
conflict with pending patches.


Thanks Adrian. I have put this in my tree and made an additional change 
along the same lines (your patched introduced an unused function warning 
to the non-debug build). If Ulrich signifies acceptance, I will send 
this on to John.


I have also sent in a patch to add a MAINTAINERS entry for zd1211rw, in 
hope that this will help you send patches with myself and/or Ulrich CC'd 
in future :)


Thanks.
Daniel
-
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 3/7] NetLabel: CIPSOv4 engine

2006-07-16 Thread Paul Moore
On Friday 14 July 2006 10:03 pm, James Morris wrote:
 On Fri, 14 Jul 2006, [EMAIL PROTECTED] wrote:
  +/**
  + * cipso_v4_bitmap_walk - Walk a bitmap looking for a bit
 
  + * cipso_v4_bitmap_setbit - Sets a single bit in a bitmap

 Can you use lib/bitmap.c instead?

Looking again at include/asm/bitops.h I think I now remember why I decided not 
to use them in the first place.  At least on the x86 side it looks like the 
underlying btsl instruction may access an entire word when the bitmap may 
only by a byte long.  I imagine in the practical sense it probably wouldn't 
be too serious of an issue, but there are too many corner cases here for me 
to be able to say for certain so I'll defer to the experts here.

If it is safe it shouldn't be too difficult to use the set/clear_bit() 
functions in cipso_v4_bitmap_setbit() and the find_*_bit() functions in 
cipso_v4_bitmap_walk().

As an aside, I've made the other changes you suggested but I won't have a 
chance to test them until Monday morning.  Assuming all goes well during some 
quick testing I'll post a new patchset around mid-day on Monday.

-- 
paul moore
linux security @ hp
-
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: [2.6.18-rc2][e1000][swsusp] - Regression - Suspend to disk and resume breaks e1000

2006-07-16 Thread Auke Kok

Shawn Starr wrote:

Hardware

IBM ThinkPad T42
E1000 card info: 

02:01.0 Ethernet controller: Intel Corporation 82540EP Gigabit Ethernet 
Controller (Mobile) (rev 03)

Subsystem: IBM PRO/1000 MT Mobile Connection
Flags: bus master, 66MHz, medium devsel, latency 64, IRQ 9
Memory at c022 (32-bit, non-prefetchable) [size=128K]
Memory at c020 (32-bit, non-prefetchable) [size=64K]
I/O ports at 8000 [size=64]
[virtual] Expansion ROM at ec00 [disabled] [size=64K]
Capabilities: [dc] Power Management version 2

Steps to reproduce:

1) Suspend to disk normally
2) Resume from disk, the e1000 will show garbage for network statistics.

snip
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:83 errors:4294967254 dropped:4294967289 overruns:0 
frame:4294967268
  TX packets:76 errors:4294967282 dropped:0 overruns:0 
carrier:4294967275

  collisions:4294967289 txqueuelen:100
  RX bytes:50728 (49.5 KiB)  TX bytes:10138 (9.9 KiB)
  Base address:0x8000 Memory:c022-c024

Did something change in the driver that forgot to save the registers / not 
register back upon resumption from disk? I can't tell from the code how the 
driver knows its been brought down to S3 or S4 states.
 
A workaround is to then suspend to memory and resume, the e1000 will work 
again. This is repeatable each time.


Not sure if anyone else noticed this.


[adding netdev to the cc]


unfortunately I didn't.

e1000 has a special e1000_pci_save_state/e1000_pci_restore_state set of 
routines that save and restore the configuration space. the fact that it works 
for suspend to memory to me suggests that there is nothing wrong with that.


I'm surprised that the t42 comes with a PCI/PCI-X e1000, which changes the 
need for this special routine, and the routine does the exact same thing as 
pci_save_state in your case. These special routines are made to handle PCI-E 
cards properly.


Also there are no config_pm changes related to this in 2.6.18-rc2. Most of 
this code has been in the kernel for a few major releases afaik. This code 
worked fine before, so I don't rule out any suspend-related issues. You should 
certainly compare with 2.6.18-rc1 and make sure it was a regression, perhaps 
even bisect the e1000-related changes if you have the time, which is about 22 
patches or so.


I'll see if I can find out some more once I get back to work.

Auke
-
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: [2.6.18-rc2][e1000][swsusp] - Regression - Suspend to disk and resume breaks e1000

2006-07-16 Thread Shawn Starr
On Sunday 16 July 2006 12:33, Auke Kok wrote:
 [adding netdev to the cc]


 unfortunately I didn't.

 e1000 has a special e1000_pci_save_state/e1000_pci_restore_state set of
 routines that save and restore the configuration space. the fact that it
 works for suspend to memory to me suggests that there is nothing wrong with
 that.

 I'm surprised that the t42 comes with a PCI/PCI-X e1000, which changes the
 need for this special routine, and the routine does the exact same thing as
 pci_save_state in your case. These special routines are made to handle
 PCI-E cards properly.

 Also there are no config_pm changes related to this in 2.6.18-rc2. Most of
 this code has been in the kernel for a few major releases afaik. This code
 worked fine before, so I don't rule out any suspend-related issues. You
 should certainly compare with 2.6.18-rc1 and make sure it was a regression,
 perhaps even bisect the e1000-related changes if you have the time, which
 is about 22 patches or so.

 I'll see if I can find out some more once I get back to work.

 Auke

The previous kernel I was using was 2.6.17 vanilla, so between this and -git 
snapshots I'll have to see where that changed.

Thanks ,
Shawn.
-
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] Fix slab corruption with netem (2nd try)

2006-07-16 Thread Guillaume Chazarain

CONFIG_DEBUG_SLAB found the following bug:
netem_enqueue() in sch_netem.c gets a pointer inside a slab object:
struct netem_skb_cb *cb = (struct netem_skb_cb *)skb-cb;
But then, the slab object may be freed:
skb = skb_unshare(skb, GFP_ATOMIC)
cb is still pointing inside the freed skb, so here is a patch to
initialize cb later, and make it clear that initializing it sooner
is a bad idea.

[From Stephen Hemminger: leave cb unitialized in order to let gcc
complain in case of use before initialization]

Thanks.

--
Guillaume



Signed-off-by: Guillaume Chazarain [EMAIL PROTECTED]
---
 net/sched/sch_netem.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff -r 1b8d63e34819 net/sched/sch_netem.c
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -148,7 +148,8 @@ static int netem_enqueue(struct sk_buff 
 static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
-	struct netem_skb_cb *cb = (struct netem_skb_cb *)skb-cb;
+	/* We don't fill cb now as skb_unshare() may invalidate it */
+	struct netem_skb_cb *cb;
 	struct sk_buff *skb2;
 	int ret;
 	int count = 1;
@@ -200,6 +201,7 @@ static int netem_enqueue(struct sk_buff 
 		skb-data[net_random() % skb_headlen(skb)] ^= 1(net_random() % 8);
 	}
 
+	cb = (struct netem_skb_cb *)skb-cb;
 	if (q-gap == 0 		/* not doing reordering */
 	|| q-counter  q-gap 	/* inside last reordering gap */
 	|| q-reorder  get_crandom(q-reorder_cor)) {



Re: [PATCH] clear skb cb on IP input

2006-07-16 Thread Herbert Xu
On Sat, Jul 15, 2006 at 06:12:22PM -0700, David Miller wrote:
 
 But I'm beginning to think that the onus of this may in fact fall upon
 the devices, in fact.  Loopback is one of the few devices where the
 control block might not be cleared out, due to uses in the output
 path.  Devices predominantly provide a zero'd out control block in the
 skb on packet receive.

The thing is qdiscs using cb means that this method of clearing cb
before netif_rx doesn't work anymore.

In particular, even if loopback clears cb before calling netif_rx,
some qdisc could come along between netif_rx and ip_rcv and put
stuff in the cb.

The same thing can happen to any NIC in fact, as long as we allow
qdiscs to use the cb area without clearing it, ip_rcv needs to
clear it itself.

With a little bit of effort we should be able to get away with
clearing just optlen.  Whether this effort is worthwhile I don't
know :)

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
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 3/7] NetLabel: CIPSOv4 engine

2006-07-16 Thread David Miller
From: Paul Moore [EMAIL PROTECTED]
Date: Sun, 16 Jul 2006 12:10:44 -0400

 On Friday 14 July 2006 10:03 pm, James Morris wrote:
  On Fri, 14 Jul 2006, [EMAIL PROTECTED] wrote:
   +/**
   + * cipso_v4_bitmap_walk - Walk a bitmap looking for a bit
  
   + * cipso_v4_bitmap_setbit - Sets a single bit in a bitmap
 
  Can you use lib/bitmap.c instead?

 Looking again at include/asm/bitops.h I think I now remember why I
 decided not to use them in the first place.

lib/bitmap.c and the asm/bitops.h operations are two entirely
different animals.

Wrt. your asm/bitops.h concerns, is there any reason you cannot pad
out your bitmaps to be a modulo of long which is required for
those routines?

-
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] clear skb cb on IP input

2006-07-16 Thread David Miller
From: Herbert Xu [EMAIL PROTECTED]
Date: Mon, 17 Jul 2006 08:03:50 +1000

 The thing is qdiscs using cb means that this method of clearing cb
 before netif_rx doesn't work anymore.
 
 In particular, even if loopback clears cb before calling netif_rx,
 some qdisc could come along between netif_rx and ip_rcv and put
 stuff in the cb.
 
 The same thing can happen to any NIC in fact, as long as we allow
 qdiscs to use the cb area without clearing it, ip_rcv needs to
 clear it itself.

Ok, I'm convinced that IPv4 has to clear this out and shouldn't
assume it's contents are anything but garbage.

 With a little bit of effort we should be able to get away with
 clearing just optlen.  Whether this effort is worthwhile I don't
 know :)

It all sits in the same cacheline, so probably not.
-
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] WAN: Added missing netif_dormant_off() to generic HDLC

2006-07-16 Thread David Miller
From: Krzysztof Halasa [EMAIL PROTECTED]
Date: Sun, 16 Jul 2006 14:17:20 +0200

 WAN: Fixed a problem with PPP/raw HDLC/X.25 protocols not doing
 netif_dormant_off() at startup.
 
 Signed-off-by: Krzysztof Halasa [EMAIL PROTECTED]

Applied, thanks.
-
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] WAN: Cosmetic changes to N2 and C101 drivers

2006-07-16 Thread David Miller
From: Krzysztof Halasa [EMAIL PROTECTED]
Date: Sun, 16 Jul 2006 14:19:07 +0200

 WAN: Cosmetic changes to N2 and C101 drivers
 
 Signed-off-by: Krzysztof Halasa [EMAIL PROTECTED]

Applied, thanks.
-
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] WAN: Converted synclink drivers to use netif_carrier_*()

2006-07-16 Thread David Miller
From: Krzysztof Halasa [EMAIL PROTECTED]
Date: Sun, 16 Jul 2006 14:23:49 +0200

 WAN: Converted synclink drivers to use netif_carrier_*() instead
 of hdlc_set_carrier().
 
 Signed-off-by: Krzysztof Halasa [EMAIL PROTECTED]

Also applied, thanks for fixing this up Krzysztof.
-
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] Fix slab corruption with netem (2nd try)

2006-07-16 Thread David Miller
From: Guillaume Chazarain [EMAIL PROTECTED]
Date: Sun, 16 Jul 2006 23:56:31 +0200

 CONFIG_DEBUG_SLAB found the following bug:
 netem_enqueue() in sch_netem.c gets a pointer inside a slab object:
 struct netem_skb_cb *cb = (struct netem_skb_cb *)skb-cb;
 But then, the slab object may be freed:
 skb = skb_unshare(skb, GFP_ATOMIC)
 cb is still pointing inside the freed skb, so here is a patch to
 initialize cb later, and make it clear that initializing it sooner
 is a bad idea.
 
 [From Stephen Hemminger: leave cb unitialized in order to let gcc
 complain in case of use before initialization]

Looks good to me, applied, thanks a lot.
-
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] clear skb cb on IP input

2006-07-16 Thread David Miller
From: chas williams - CONTRACTOR [EMAIL PROTECTED]
Date: Sun, 16 Jul 2006 07:20:35 -0400

 why does the input side of the ip layer believe the cb contains valid
 data?  it should zero the contents of the cb, or just fill in the cb
 correctly when the packet arrives at its doorstop.

Yes, that's right, after hearing your and Herbert's comments,
this point of view is clearly right :)
-
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 3/7] NetLabel: CIPSOv4 engine

2006-07-16 Thread Paul Moore
On Sunday 16 July 2006 9:12 pm, David Miller wrote:
 From: Paul Moore [EMAIL PROTECTED]
 Date: Sun, 16 Jul 2006 12:10:44 -0400

  On Friday 14 July 2006 10:03 pm, James Morris wrote:
   On Fri, 14 Jul 2006, [EMAIL PROTECTED] wrote:
+/**
+ * cipso_v4_bitmap_walk - Walk a bitmap looking for a bit
   
+ * cipso_v4_bitmap_setbit - Sets a single bit in a bitmap
  
   Can you use lib/bitmap.c instead?
 
  Looking again at include/asm/bitops.h I think I now remember why I
  decided not to use them in the first place.

 lib/bitmap.c and the asm/bitops.h operations are two entirely
 different animals.

I probably should have been more clear - I didn't see anything in lib/bitmap.c 
(or include/linux/bitmap.h) that I think would have been useful.  However, 
include/linux/bitmap.h makes reference to asm/bitops.h which does have some 
function which may have been useful if they didn't have the length 
restrictions.

 Wrt. your asm/bitops.h concerns, is there any reason you cannot pad
 out your bitmaps to be a modulo of long which is required for
 those routines?

Right now I use both the bitmap_walk() and bitmap_setbit() routines to deal 
with both CIPSO tags straight from the sk_buff as well as the internal bitmap 
representation.  Padding out the internal bitmaps would require some code 
changes but there isn't much I can do about the packet I don't believe.  True 
it would probably be okay for most packets to assume you could access an 
entire longs worth of memory but then again it would only take one evil 
packet to start causing problems ...

-- 
paul moore
linux security @ hp
-
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 3/7] NetLabel: CIPSOv4 engine

2006-07-16 Thread David Miller
From: Paul Moore [EMAIL PROTECTED]
Date: Sun, 16 Jul 2006 22:42:07 -0400

 Right now I use both the bitmap_walk() and bitmap_setbit() routines
 to deal with both CIPSO tags straight from the sk_buff as well as
 the internal bitmap representation.  Padding out the internal
 bitmaps would require some code changes but there isn't much I can
 do about the packet I don't believe.  True it would probably be okay
 for most packets to assume you could access an entire longs worth
 of memory but then again it would only take one evil packet to start
 causing problems ...

The access also has to be long aligned to be valid for
the asm/bitops.h interfaces as well, which is also
problematic for your case I guess.
-
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] Bug in pskb_trim_rcsum()

2006-07-16 Thread Wei Yongjun
  Since network device can auto calculate and verify the checksum of a
packet, for example: some e1000 interface. Different device will set
different value of skb-ip_summed.
  a) If device do nothing to checksum, skb-ip_summed would be set to
CHECKSUM_NONE.
  b) If device can only calculate a checksum, and the checksum is
correct, skb-ip_summed would be set to CHECKSUM_HW.
  c) If device can verify the checksum, and the checksum is correct,
skb-ip_summed would be set to CHECKSUM_UNNECESSARY.
  So if I want to trim a skb, I think I must do a checksum even if the
skb-ip_summed is CHECKSUM_UNNECESSARY.

Following is the comment about CHECKSUM_UNNECESSARY in
include/linux/skbuff.h:
 *  UNNECESSARY: device parsed packet and wouldbe verified checksum.
 *  skb-csum is undefined.
 *It is bad option, but, unfortunately, many of vendors do this.
 *Apparently with secret goal to sell you new device, when you
 *will add new protocol to your host. F.e. IPv6. 8)

Signed-off-by: Wei Yongjun [EMAIL PROTECTED]


--- a/include/linux/skbuff.h2006-07-17 10:14:23.175070472 -0400
+++ b/include/linux/skbuff.h2006-07-17 10:18:31.762279472 -0400
@@ -1208,8 +1208,8 @@ static inline int pskb_trim_rcsum(struct
 {
if (likely(len = skb-len))
return 0;
-   if (skb-ip_summed == CHECKSUM_HW)
-   skb-ip_summed = CHECKSUM_NONE;
+
+   skb-ip_summed = CHECKSUM_NONE;
return __pskb_trim(skb, len);
 }
 


-
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