[PATCH 2/2] mISDN: fix OOM condition for sending queued I-Frames

2015-10-21 Thread Karsten Keil
The old code did not check the return value of skb_clone().
The extra skb_clone() is not needed at all, if using skb_realloc_headroom()
instead, which gives us a private copy with enough headroom as well.
We need to requeue the original skb if the call failed, because we cannot
inform upper layers about the data loss. Restructure the code to minimise
rollback effort if it happens.
This fix kernel bug #86091

Thanks to Insu Yun <wuni...@gmail.com> to remind me on this issue.

Signed-off-by: Karsten Keil <k...@b1-systems.de>
---
 drivers/isdn/mISDN/layer2.c | 54 +
 1 file changed, 20 insertions(+), 34 deletions(-)

diff --git a/drivers/isdn/mISDN/layer2.c b/drivers/isdn/mISDN/layer2.c
index 949cabb..5eb380a 100644
--- a/drivers/isdn/mISDN/layer2.c
+++ b/drivers/isdn/mISDN/layer2.c
@@ -1476,7 +1476,7 @@ static void
 l2_pull_iqueue(struct FsmInst *fi, int event, void *arg)
 {
struct layer2   *l2 = fi->userdata;
-   struct sk_buff  *skb, *nskb, *oskb;
+   struct sk_buff  *skb, *nskb;
u_char  header[MAX_L2HEADER_LEN];
u_int   i, p1;
 
@@ -1486,48 +1486,34 @@ l2_pull_iqueue(struct FsmInst *fi, int event, void *arg)
skb = skb_dequeue(>i_queue);
if (!skb)
return;
-
-   if (test_bit(FLG_MOD128, >flag))
-   p1 = (l2->vs - l2->va) % 128;
-   else
-   p1 = (l2->vs - l2->va) % 8;
-   p1 = (p1 + l2->sow) % l2->window;
-   if (l2->windowar[p1]) {
-   printk(KERN_WARNING "%s: l2 try overwrite ack queue entry %d\n",
-  mISDNDevName4ch(>ch), p1);
-   dev_kfree_skb(l2->windowar[p1]);
-   }
-   l2->windowar[p1] = skb;
i = sethdraddr(l2, header, CMD);
if (test_bit(FLG_MOD128, >flag)) {
header[i++] = l2->vs << 1;
header[i++] = l2->vr << 1;
+   } else
+   header[i++] = (l2->vr << 5) | (l2->vs << 1);
+   nskb = skb_realloc_headroom(skb, i);
+   if (!nskb) {
+   printk(KERN_WARNING "%s: no headroom(%d) copy for IFrame\n",
+  mISDNDevName4ch(>ch), i);
+   skb_queue_head(>i_queue, skb);
+   return;
+   }
+   if (test_bit(FLG_MOD128, >flag)) {
+   p1 = (l2->vs - l2->va) % 128;
l2->vs = (l2->vs + 1) % 128;
} else {
-   header[i++] = (l2->vr << 5) | (l2->vs << 1);
+   p1 = (l2->vs - l2->va) % 8;
l2->vs = (l2->vs + 1) % 8;
}
-
-   nskb = skb_clone(skb, GFP_ATOMIC);
-   p1 = skb_headroom(nskb);
-   if (p1 >= i)
-   memcpy(skb_push(nskb, i), header, i);
-   else {
-   printk(KERN_WARNING
-  "%s: L2 pull_iqueue skb header(%d/%d) too short\n",
-  mISDNDevName4ch(>ch), i, p1);
-   oskb = nskb;
-   nskb = mI_alloc_skb(oskb->len + i, GFP_ATOMIC);
-   if (!nskb) {
-   dev_kfree_skb(oskb);
-   printk(KERN_WARNING "%s: no skb mem in %s\n",
-  mISDNDevName4ch(>ch), __func__);
-   return;
-   }
-   memcpy(skb_put(nskb, i), header, i);
-   memcpy(skb_put(nskb, oskb->len), oskb->data, oskb->len);
-   dev_kfree_skb(oskb);
+   p1 = (p1 + l2->sow) % l2->window;
+   if (l2->windowar[p1]) {
+   printk(KERN_WARNING "%s: l2 try overwrite ack queue entry %d\n",
+  mISDNDevName4ch(>ch), p1);
+   dev_kfree_skb(l2->windowar[p1]);
}
+   l2->windowar[p1] = skb;
+   memcpy(skb_push(nskb, i), header, i);
l2down(l2, PH_DATA_REQ, l2_newid(l2), nskb);
test_and_clear_bit(FLG_ACK_PEND, >flag);
if (!test_and_set_bit(FLG_T200_RUN, >flag)) {
-- 
2.1.4

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


[PATCH 0/2] Fix potential NULL pointer access and memory leak in ISDN layer2 functions

2015-10-21 Thread Karsten Keil
Insu Yun did brinup the issue with not checking the skb_clone() return
value in the layer2 I-frame ull functions.
This series fix the issue in a way which avoid protocol violations/data loss
on a temporary memory shortage.

Karsten Keil (2):
  ISDN: fix OOM condition for sending queued I-Frames
  mISDN: fix OOM condition for sending queued I-Frames

 drivers/isdn/hisax/isdnl2.c | 20 +++--
 drivers/isdn/mISDN/layer2.c | 54 +
 2 files changed, 28 insertions(+), 46 deletions(-)

-- 
2.1.4

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


[PATCH 1/2] ISDN: fix OOM condition for sending queued I-Frames

2015-10-21 Thread Karsten Keil
The skb_clone() return value was not checked and the skb_realloc_headroom()
usage was wrong, the old skb was not freed. It turned out, that the
skb_clone is not needed at all, the skb_realloc_headroom() will create a
private copy with enough headroom and the original SKB can be used for the
ACK queue.
We need to requeue the original skb if the call failed, since the upper
layer cannot be informed about memory shortage.

Thanks to Insu Yun <wuni...@gmail.com> to remind me on this issue.

Signed-off-by: Karsten Keil <k...@b1-systems.de>
---
 drivers/isdn/hisax/isdnl2.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/isdn/hisax/isdnl2.c b/drivers/isdn/hisax/isdnl2.c
index 18accb0..c53a53f 100644
--- a/drivers/isdn/hisax/isdnl2.c
+++ b/drivers/isdn/hisax/isdnl2.c
@@ -1247,7 +1247,7 @@ static void
 l2_pull_iqueue(struct FsmInst *fi, int event, void *arg)
 {
struct PStack *st = fi->userdata;
-   struct sk_buff *skb;
+   struct sk_buff *skb, *nskb;
struct Layer2 *l2 = >l2;
u_char header[MAX_HEADER_LEN];
int i, hdr_space_needed;
@@ -1262,14 +1262,10 @@ l2_pull_iqueue(struct FsmInst *fi, int event, void *arg)
return;
 
hdr_space_needed = l2headersize(l2, 0);
-   if (hdr_space_needed > skb_headroom(skb)) {
-   struct sk_buff *orig_skb = skb;
-
-   skb = skb_realloc_headroom(skb, hdr_space_needed);
-   if (!skb) {
-   dev_kfree_skb(orig_skb);
-   return;
-   }
+   nskb = skb_realloc_headroom(skb, hdr_space_needed);
+   if (!nskb) {
+   skb_queue_head(>i_queue, skb);
+   return;
}
spin_lock_irqsave(>lock, flags);
if (test_bit(FLG_MOD128, >flag))
@@ -1282,7 +1278,7 @@ l2_pull_iqueue(struct FsmInst *fi, int event, void *arg)
   p1);
dev_kfree_skb(l2->windowar[p1]);
}
-   l2->windowar[p1] = skb_clone(skb, GFP_ATOMIC);
+   l2->windowar[p1] = skb;
 
i = sethdraddr(>l2, header, CMD);
 
@@ -1295,8 +1291,8 @@ l2_pull_iqueue(struct FsmInst *fi, int event, void *arg)
l2->vs = (l2->vs + 1) % 8;
}
spin_unlock_irqrestore(>lock, flags);
-   memcpy(skb_push(skb, i), header, i);
-   st->l2.l2l1(st, PH_PULL | INDICATION, skb);
+   memcpy(skb_push(nskb, i), header, i);
+   st->l2.l2l1(st, PH_PULL | INDICATION, nskb);
test_and_clear_bit(FLG_ACK_PEND, >l2.flag);
if (!test_and_set_bit(FLG_T200_RUN, >l2.flag)) {
FsmDelTimer(>l2.t203, 13);
-- 
2.1.4

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


Re: Linux IPv6 DAD not full conform to RFC 4862 ?

2008-01-10 Thread Karsten Keil
Hi,
On Wed, Jan 09, 2008 at 09:26:53PM +0100, Karsten Keil wrote:
  
  Reading the section you reference, we do follow all the MUST requirements, 
  and
  we log an error.  Given that the disable section is a SHOULD, I think we 
  can at
  least be somewhat more restrictive in our implementation.  Perhaps we should
  just disable the interface iff the failed address is link-local AND there 
  are no
  other functional address assigned to the interface.
 
 I agree here, but it seems that currently the IPv6 Logo Committee thinks
 that it has to be disable the interface to get the IPv6 ready Logo in
 future. I already claim that on a discussion at the TAHI users list.
 

JFYI, here the answer from the TAHI list.

Hi, Karsten.

Thanks for your comments.

I know that it is SHOULD,
but our test tool supports the test specification
published by IPv6 Ready Logo Program http://www.ipv6ready.org/,
and basically the test specification supports all of MUST and SHOULD.

You may know,
now IPv6 Ready Logo Committee is also discussing
about the next major revision up of test specification.


RFC 4862 Section 5.4.5 is one of discussing point.

The public review has been over,
but if you have strong concern about it,
I recommend to comment to [EMAIL PROTECTED].

Personally,
I think that mandating this function is the best way.
But vendor's input will really important for them.

Regards,
Yukiyo Akisada


So it would be good if some of the networking experts complain there.

-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
--
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: Linux IPv6 DAD not full conform to RFC 4862 ?

2008-01-10 Thread Karsten Keil
On Wed, Jan 09, 2008 at 03:32:12PM -0800, David Miller wrote:
 From: Karsten Keil [EMAIL PROTECTED]
 Date: Wed, 9 Jan 2008 16:36:56 +0100
 
 If the address is a link-local address formed from an interface
 identifier based on the hardware address, which is supposed to be
 uniquely assigned (e.g., EUI-64 for an Ethernet interface), IP
 operation on the interface SHOULD be disabled.  By disabling IP
 operation, the node will then:
  
 -  not send any IP packets from the interface,
  
 -  silently drop any IP packets received on the interface, and
  
 -  not forward any IP packets to the interface (when acting as a
router or processing a packet with a Routing header).
 
 I question any RFC mandate that shuts down IP communication on a node
 because of packets received from remote systems.
 
 If the TAHI test can trigger this, so can a compromised system on your
 network and won't that be fun? :-)

I agree, but on the other side, a interface with a real duplicate HW address
sending packets on the network can also cause very serious problems, and
maybe is not so easy to detect as  a system where the interface never come
up because of this. So maybe it makes sense to implement it as option, not
as default.
And the DOS scenario is already here, also without disabling IP completely,
since you can deny any IPv6 address assignments with faked DAD pakets.

-- 
Karsten Keil
SuSE Labs
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
--
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


Linux IPv6 DAD not full conform to RFC 4862 ?

2008-01-09 Thread Karsten Keil
Hi,

I tried to run the 1.5.0 Beta2  TAHI Selftest on recent Linux kernel.
It fails in the Stateless Address Autoconfiguration section with
6 tests.
These tests are for Duplicate Address Detection (DAD).
They are detect for the Link Local Address a duplicate address on the
network. It seems that our current behavior is to log an message and
do not assign this address.

But the RFC 4862 says:

5.4.5.  When Duplicate Address Detection Fails

   A tentative address that is determined to be a duplicate as described
   above MUST NOT be assigned to an interface, and the node SHOULD log a
   system management error.

   If the address is a link-local address formed from an interface
   identifier based on the hardware address, which is supposed to be
   uniquely assigned (e.g., EUI-64 for an Ethernet interface), IP
   operation on the interface SHOULD be disabled.  By disabling IP
   operation, the node will then:

   -  not send any IP packets from the interface,

   -  silently drop any IP packets received on the interface, and

   -  not forward any IP packets to the interface (when acting as a
  router or processing a packet with a Routing header).

   In this case, the IP address duplication probably means duplicate
   hardware addresses are in use, and trying to recover from it by
   configuring another IP address will not result in a usable network.
   In fact, it probably makes things worse by creating problems that are
   harder to diagnose than just disabling network operation on the
   interface; the user will see a partially working network where some
   things work, and other things do not.

   On the other hand, if the duplicate link-local address is not formed
   from an interface identifier based on the hardware address, which is
   supposed to be uniquely assigned, IP operation on the interface MAY
   be continued.


So I think we should disable the interface now, if DAD fails on a
hardware based LLA.

-- 
Karsten Keil
SuSE Labs
--
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: Linux IPv6 DAD not full conform to RFC 4862 ?

2008-01-09 Thread Karsten Keil
On Wed, Jan 09, 2008 at 11:17:48AM -0500, Neil Horman wrote:
 On Wed, Jan 09, 2008 at 04:36:56PM +0100, Karsten Keil wrote:
  Hi,
  
  I tried to run the 1.5.0 Beta2  TAHI Selftest on recent Linux kernel.
  It fails in the Stateless Address Autoconfiguration section with
  6 tests.
  These tests are for Duplicate Address Detection (DAD).
  They are detect for the Link Local Address a duplicate address on the
  network. It seems that our current behavior is to log an message and
  do not assign this address.
  
  But the RFC 4862 says:
  
  5.4.5.  When Duplicate Address Detection Fails
  
 A tentative address that is determined to be a duplicate as described
 above MUST NOT be assigned to an interface, and the node SHOULD log a
 system management error.
  
 If the address is a link-local address formed from an interface
 identifier based on the hardware address, which is supposed to be
 uniquely assigned (e.g., EUI-64 for an Ethernet interface), IP
 operation on the interface SHOULD be disabled.  By disabling IP
 operation, the node will then:
  
 -  not send any IP packets from the interface,
  
 -  silently drop any IP packets received on the interface, and
  
 -  not forward any IP packets to the interface (when acting as a
router or processing a packet with a Routing header).
  
 In this case, the IP address duplication probably means duplicate
 hardware addresses are in use, and trying to recover from it by
 configuring another IP address will not result in a usable network.
 In fact, it probably makes things worse by creating problems that are
 harder to diagnose than just disabling network operation on the
 interface; the user will see a partially working network where some
 things work, and other things do not.
  
 On the other hand, if the duplicate link-local address is not formed
 from an interface identifier based on the hardware address, which is
 supposed to be uniquely assigned, IP operation on the interface MAY
 be continued.
  
  
  So I think we should disable the interface now, if DAD fails on a
  hardware based LLA.
  
 
 Not sure I agree with that.  I assume that by disable, you mean that we should
 clear the IFF_UP flag?  If we do that, and another ip address is assigned to
 that interface, then your proposal would discontinue the functionality of 
 those
 already established addresses, which would be bad.  I could see a DOS scenario
 comming out of that as well.  Simply send ndisc na's for a recently advertised
 address, and you could prevent network communication for an entire system.
 
 Reading the section you reference, we do follow all the MUST requirements, and
 we log an error.  Given that the disable section is a SHOULD, I think we can 
 at
 least be somewhat more restrictive in our implementation.  Perhaps we should
 just disable the interface iff the failed address is link-local AND there are 
 no
 other functional address assigned to the interface.

I agree here, but it seems that currently the IPv6 Logo Committee thinks
that it has to be disable the interface to get the IPv6 ready Logo in
future. I already claim that on a discussion at the TAHI users list.


So far I remember a SHOULD in RFC has to interpreted as You should
implement that in this way, exceptions are only acceptable for a good
reason. Maybe the DOS scenario is such a good reason.

-- 
Karsten Keil
SuSE Labs
--
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: Linux IPv6 DAD not full conform to RFC 4862 ?

2008-01-09 Thread Karsten Keil
On Thu, Jan 10, 2008 at 01:40:51AM +0900, YOSHIFUJI Hideaki / 吉藤英明 wrote:
 In article [EMAIL PROTECTED] (at Thu, 10 Jan 2008 01:38:57 +0900 (JST)), 
 YOSHIFUJI Hideaki / 吉藤英明 [EMAIL PROTECTED] says:
 
  - we could have dad_reaction interface variable and
1: disable interface
   = 1: disable IPv6
0: ignore (as we do now)
 
 Argh, 0, 0 and 0, maybe.
 

I would like this solution, I had something similar to this in mind after I
discovered the issue.

-- 
Karsten Keil
SuSE Labs
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
--
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: Why does a connect to IPv6 LLA address fail ?

2007-11-08 Thread Karsten Keil
On Thu, Nov 08, 2007 at 01:15:52PM -0500, Vlad Yasevich wrote:
 Andreas Gruenbacher wrote:
  On Wednesday 07 November 2007 20:42, Vlad Yasevich wrote:
  The reason is that 2 different hosts may have the same link-local
  address as long as they are on different links.  If the sender is
  connected to both links then it may send the packet to the wrong
  destination.
  
  Good point.
  
  What's confusing is that connect(2) fails even if the host itself has the 
  specified address. This isn't necessary.
 
 Yes and no.  Since linux doesn't have the concept of default zone, we have
 to fail, because from the perspective of the kernel, the address was not
 fully specified.  OTOH, since this is our address, we 'could' have all
 the info.
 

 The problem is that this verification happens before we hit the routing logic.
 It's an explicit check the if the sin6_scope_id is not set and we are not 
 bound
 to an interface, it's an error.
 

OK I run into this issue while running the TAHI testsuite. The test is as
follows:

  Check 03:
DNS Address: fec0::9
Candidate Source Addresses: fec0::1(SS) or LLA(LS)
Destination Address List: 3fff::2(GS) or fe80::2(LS)
Result: fe80::2 (src LLA) then 3fff::2 (src fec0::1)

Scope(fe80::2) = Scope(LLA) and Scope(3fff::2)  Scope(fec0::1), then 
prefer fe80::2

the nameserver send following answer for the query:

| | | | DNS_Question(length:21)
| | | | | DNS_QuestionEntry   (length:21)
| | | | | | Name = server.tahi.org.
| | | | | | Type = 28 ()
| | | | | | Class= 1
| | | | DNS_Answer  (length:86)
| | | | | DNS_RR_ (length:43)
| | | | | | Name = server.tahi.org.
| | | | | | Type = 28
| | | | | | Class= 1
| | | | | | TTL  = 0
| | | | | | Length   = 16
| | | | | | Address  = 3fff::2
| | | | | DNS_RR_ (length:43)
| | | | | | Name = server.tahi.org.
| | | | | | Type = 28
| | | | | | Class= 1
| | | | | | TTL  = 0
| | | | | | Length   = 16
| | | | | | Address  = fe80::2



So how we should handle this issue, claim that the test is wrong, the test
should not use LLA for this ?

-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
-
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


Why does a connect to IPv6 LLA address fail ?

2007-11-07 Thread Karsten Keil
Hi,

currently I do some cerification test for IPv6 with the TAHI ct testsuite.
With the default-addr-select tests for compliance with RFC3484 here are
FAILs with Destination Address Selection Check Rule 2(Prefer matching
scope). Yes I know that Destination Address Selection is done in glibc,
but it seems that the kernel behaves wrong in the connect system call
with IPv6 link local addresses. 

The glibc getaddrinfo function try to verify if a address is valid and
examine the source address. For this it create a socket for datagram and
protocol IPPROTO_IP and then try to connect it with the destination
address. This fails in the case of a LLA, because connect returns EINVAL,
since here is no device bind to this socket at this time. So getaddrinfo
mark this candidate address as not reachable and so it will never prefered
because of rule 1 of RFC3484 Destination Address Selection.

Why do we have this check in ip6_datagram_connect() ?

The posix manpage for connect says about EINVAL:
EINVAL - The address_len argument is not a valid length for the address 
family; or
invalid address family in the sockaddr structure.

Which is not the case here.

-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
-
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] isdn compile fix (resend)

2007-10-15 Thread Karsten Keil
On Mon, Oct 15, 2007 at 06:44:56PM +0400, Denis V. Lunev wrote:
Compilation fix. The problem appears after
7c076d1de869256848dacb8de0050a3a390f95df by Karsten Keil [EMAIL PROTECTED]

Acked-by: Karsten Keil [EMAIL PROTECTED]
Signed-off-by: Denis V. Lunev [EMAIL PROTECTED]

--- ./drivers/isdn/i4l/isdn_net.c.isdn2 2007-10-15 13:55:24.0 +0400
+++ ./drivers/isdn/i4l/isdn_net.c   2007-10-15 18:19:11.0 +0400
@@ -2713,7 +2713,7 @@ isdn_net_setcfg(isdn_net_ioctl_cfg * cfg
case ISDN_NET_ENCAP_X25IFACE:
 #ifndef CONFIG_ISDN_X25
printk(KERN_WARNING %s: isdn-x25 support not 
configured\n,
-  p-local-name);
+  p-dev-name);
return -EINVAL;
 #else
p-dev-type = ARPHRD_X25;  /* change ARP type */

-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
-
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.23-rc8-mm2 BUG: register_netdevice() issue as (ab)used by ISDN

2007-10-12 Thread Karsten Keil
Hi Dave,

a follow up to the previous patch for net-2.6.24.

On Mon, Oct 08, 2007 at 08:37:33PM -0700, David Miller wrote:
  
  You could try following patch with 2.6.23-rc8-mm2, it change I4L to use
  alloc_netdev().
  
  Signed-off-by: Karsten Keil [EMAIL PROTECTED]
 
 I've added this patch to net-2.6.24, thanks Karsten!

This is a followup to the previous ISDN alloc_netdev() patch.
It removes the local copy of the device name to make sure
that device renames work.

Signed-off-by: Karsten Keil [EMAIL PROTECTED]

diff -urNp linux-2.6.23.old/drivers/isdn/i4l/isdn_net.c 
linux-2.6.23/drivers/isdn/i4l/isdn_net.c
--- linux-2.6.23.old/drivers/isdn/i4l/isdn_net.c2007-10-08 
17:31:43.0 +0200
+++ linux-2.6.23/drivers/isdn/i4l/isdn_net.c2007-10-12 13:43:50.0 
+0200
@@ -328,7 +328,7 @@ isdn_net_autohup(void)
l-cps = (l-transcount * HZ) / (jiffies - 
last_jiffies);
l-transcount = 0;
if (dev-net_verbose  3)
-   printk(KERN_DEBUG %s: %d bogocps\n, l-name, l-cps);
+   printk(KERN_DEBUG %s: %d bogocps\n, p-dev-name, 
l-cps);
if ((l-flags  ISDN_NET_CONNECTED)  (!l-dialstate)) {
anymore = 1;
l-huptimer++;
@@ -350,12 +350,12 @@ isdn_net_autohup(void)
if (l-hupflags  ISDN_CHARGEHUP) {
if (l-hupflags  
ISDN_WAITCHARGE) {
printk(KERN_DEBUG 
isdn_net: Hupflags of %s are %X\n,
-  l-name, 
l-hupflags);
+  p-dev-name, 
l-hupflags);
isdn_net_hangup(p-dev);
} else if (time_after(jiffies, 
l-chargetime + l-chargeint)) {
printk(KERN_DEBUG
   isdn_net: %s: 
chtime = %lu, chint = %d\n,
-  l-name, 
l-chargetime, l-chargeint);
+  p-dev-name, 
l-chargetime, l-chargeint);
isdn_net_hangup(p-dev);
}
} else
@@ -442,8 +442,8 @@ isdn_net_stat_callback(int idx, isdn_ctr
 #endif
isdn_net_lp_disconnected(lp);
isdn_all_eaz(lp-isdn_device, 
lp-isdn_channel);
-   printk(KERN_INFO %s: remote hangup\n, 
lp-name);
-   printk(KERN_INFO %s: Chargesum is 
%d\n, lp-name,
+   printk(KERN_INFO %s: remote hangup\n, 
p-dev-name);
+   printk(KERN_INFO %s: Chargesum is 
%d\n, p-dev-name,
   lp-charge);
isdn_net_unbind_channel(lp);
return 1;
@@ -487,7 +487,7 @@ isdn_net_stat_callback(int idx, isdn_ctr

isdn_net_add_to_bundle(nd, lp);
}
}
-   printk(KERN_INFO isdn_net: %s 
connected\n, lp-name);
+   printk(KERN_INFO isdn_net: %s 
connected\n, p-dev-name);
/* If first Chargeinfo comes 
before B-Channel connect,
 * we correct the timestamp 
here.
 */
@@ -534,7 +534,7 @@ isdn_net_stat_callback(int idx, isdn_ctr
lp-hupflags |= ISDN_HAVECHARGE;
lp-chargetime = jiffies;
printk(KERN_DEBUG isdn_net: Got CINF 
chargetime of %s now %lu\n,
-  lp-name, lp-chargetime);
+  p-dev-name, lp-chargetime);
return 1;
}
}
@@ -565,7 +565,7 @@ isdn_net_dial(void)
 
 #ifdef ISDN_DEBUG_NET_DIAL
if (lp-dialstate)
-   printk(KERN_DEBUG %s: dialstate=%d\n, lp-name, 
lp-dialstate);
+   printk(KERN_DEBUG %s: dialstate=%d\n, p-dev-name, 
lp-dialstate);
 #endif
switch (lp-dialstate) {
case 0:
@@ -578,7 +578,7 @@ isdn_net_dial(void)
lp-dial = lp-phone[1

Re: 2.6.23-rc8-mm2 BUG: register_netdevice() issue as (ab)used by ISDN

2007-10-08 Thread Karsten Keil
On Sun, Oct 07, 2007 at 02:06:53PM +0200, Andreas Mohr wrote:
 [not necessarily a very recent regression, used 2.6.19 kernels before...]
 
 Hi all,
 
 wondered why my main internet server (headless!) didn't come up properly
 on a new 2.6.23-rc8-mm2
...
 as if it's the
BUG_ON(!dev-nd_net);
 check which caused the BUG message.

The regression comes from the net namespace patches.
Up to now in ISDN the struct netdevice is contained in struct isdn_net_dev and 
so
it's not allocated using alloc_netdev(), which would set the missing dev-nd_net
pointer.


-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
-
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.23-rc8-mm2 BUG: register_netdevice() issue as (ab)used by ISDN

2007-10-08 Thread Karsten Keil
On Sun, Oct 07, 2007 at 07:20:37PM +0200, Andreas Mohr wrote:
 Hi,
 
 On Sun, Oct 07, 2007 at 03:17:24PM +0200, Andreas Mohr wrote:
  I thus decided to now try plain 2.6.23-rc8 whether it's corrupted, too.
 
 OK, after 3 hours of compilation the ONLY datapoint I can give right now is:
 1 (one) boot on 2.6.23-rc8-mm2 had the BUG in /var/log/messages,
 1 (one) boot on 2.6.23-rc8 did NOT have it.
 
 The two .config:s (I did a make oldconfig when going back to
 non-mm2) don't seem to have any relevant differences.
 
 I could offer to do more testing on this machine,
 but this carries a slight risk for me since I (personally) won't have physical
 access to it for a couple days and some people will kill me if the main
 internet gateway happened to go down unfixably. ;)


You could try following patch with 2.6.23-rc8-mm2, it change I4L to use
alloc_netdev().

Signed-off-by: Karsten Keil [EMAIL PROTECTED]

diff -ur linux-2.6.23-rc9.org/drivers/isdn/i4l/isdn_net.c 
linux-2.6.23-rc9/drivers/isdn/i4l/isdn_net.c
--- linux-2.6.23-rc9.org/drivers/isdn/i4l/isdn_net.c2007-10-08 
12:01:22.0 +0200
+++ linux-2.6.23-rc9/drivers/isdn/i4l/isdn_net.c2007-10-08 
17:31:43.0 +0200
@@ -77,7 +77,7 @@
if (lp-master) 
dev = lp-master;
else
-   dev = n-dev;
+   dev = n-dev;
return netif_running(dev);
 }
 
@@ -90,7 +90,7 @@
if (lp-master) 
netif_wake_queue(lp-master);
else
-   netif_wake_queue(lp-netdev-dev);
+   netif_wake_queue(lp-netdev-dev);
 }
 
 /*
@@ -102,7 +102,7 @@
if (lp-master)
netif_stop_queue(lp-master);
else
-   netif_stop_queue(lp-netdev-dev);
+   netif_stop_queue(lp-netdev-dev);
 }
 
 /*
@@ -287,7 +287,7 @@
   BEWARE! This chunk of code cannot be called from hardware
   interrupt handler. I hope it is true. --ANK
 */
-   qdisc_reset(lp-netdev-dev.qdisc);
+   qdisc_reset(lp-netdev-dev-qdisc);
}
lp-dialstate = 0;
dev-rx_netdev[isdn_dc2minor(lp-isdn_device, lp-isdn_channel)] = NULL;
@@ -345,27 +345,27 @@
l-chargetime += l-chargeint;
if (time_after(jiffies, l-chargetime + 
l-chargeint - 2 * HZ))
if (l-outgoing || l-hupflags 
 ISDN_INHUP)
-   
isdn_net_hangup(p-dev);
+   isdn_net_hangup(p-dev);
} else if (l-outgoing) {
if (l-hupflags  ISDN_CHARGEHUP) {
if (l-hupflags  
ISDN_WAITCHARGE) {
printk(KERN_DEBUG 
isdn_net: Hupflags of %s are %X\n,
   l-name, 
l-hupflags);
-   
isdn_net_hangup(p-dev);
+   isdn_net_hangup(p-dev);
} else if (time_after(jiffies, 
l-chargetime + l-chargeint)) {
printk(KERN_DEBUG
   isdn_net: %s: 
chtime = %lu, chint = %d\n,
   l-name, 
l-chargetime, l-chargeint);
-   
isdn_net_hangup(p-dev);
+   isdn_net_hangup(p-dev);
}
} else
-   isdn_net_hangup(p-dev);
+   isdn_net_hangup(p-dev);
} else if (l-hupflags  ISDN_INHUP)
-   isdn_net_hangup(p-dev);
+   isdn_net_hangup(p-dev);
}
 
if(dev-global_flags  ISDN_GLOBAL_STOPPED || 
(ISDN_NET_DIALMODE(*l) == ISDN_NET_DM_OFF)) {
-   isdn_net_hangup(p-dev);
+   isdn_net_hangup(p-dev);
break;
}
}
@@ -579,7 +579,7 @@
if (!lp-dial) {
printk(KERN_WARNING %s: phone number 
deleted?\n,
   lp-name);
-   isdn_net_hangup(p-dev);
+   isdn_net_hangup(p-dev);
break

Re: [PATCH] isdn capi driver broken on 64 bit.

2007-08-28 Thread Karsten Keil
Hello Steven,

On Mon, Aug 27, 2007 at 09:16:55AM -0700, Stephen Hemminger wrote:
 On Mon, 27 Aug 2007 13:02:26 +0200
 Karsten Keil [EMAIL PROTECTED] wrote:
 
  On Fri, Aug 24, 2007 at 11:08:11AM -0700, Stephen Hemminger wrote:
   The following driver API is broken on any architecture with 64 bit 
   addresses.
   because of cast that loses high bits.
   
   Signed-off-by: Stephen Hemminger [EMAIL PROTECTED]
   
   
   --- a/drivers/isdn/capi/capidrv.c 2007-06-25 09:03:12.0 -0700
   +++ b/drivers/isdn/capi/capidrv.c 2007-08-24 11:06:46.0 -0700
   @@ -1855,6 +1855,9 @@ static int if_sendbuf(int id, int channe
 return 0;
 }
 datahandle = nccip-datahandle;
   +
   + /* This won't work on 64 bit! */
   + BUILD_BUG_ON(sizeof(skb-data)  sizeof(u32));
 capi_fill_DATA_B3_REQ(sendcmsg, global.ap.applid, card-msgid++,
   nccip-ncci,  /* adr */
   (u32) skb-data,  /* Data */
  
  
  NACK.
  
  It is not a BUG.
  
  This is OK, since this field must have a value and on 32 it has the correct
  one) On 64 bit this field is ignored (but also need a value, random data is
  bad as well).
 
 If you are using it as a transaction ID, then you should generate one.
 There is no guarantee that two skb's won't have the same 32 bit data value
 on 64 bit.

No, it's the address of the data buffer, but nobody use it in linux kernel.
A CAPI DATA_B3 message looks like this in Linux:

8 Byte CAPI HEADER
32 bit NCCI
32 bit Pointer to DataBuffer
16 bit length
16 bit handle
16 bit Flags
Start of DataBuffer


So the payload data follows directely the Message header and inside
Linux we use this to address the data. But we must fill the pointer
with some data, so we use the correct value for 32 bit, for 64 bit
it is wrong yes, but since the value is not used ..., I want to
avoid using a special case for 64 bit (e.g. setiing it to zero).
Other OS use seperate buffers for the payload data, thats the reason why
this field exist.
For 64 bit a (optional) extention exist, which place a 64 bit Pointer
beween 16 bit Flags and Start DataBuffer so you can adress seperate
data buffers under 64 as well, but we do no use that inside kernel CAPI
implementation.


-


-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
-
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] isdn capi driver broken on 64 bit.

2007-08-27 Thread Karsten Keil
On Fri, Aug 24, 2007 at 11:08:11AM -0700, Stephen Hemminger wrote:
 The following driver API is broken on any architecture with 64 bit addresses.
 because of cast that loses high bits.
 
 Signed-off-by: Stephen Hemminger [EMAIL PROTECTED]
 
 
 --- a/drivers/isdn/capi/capidrv.c 2007-06-25 09:03:12.0 -0700
 +++ b/drivers/isdn/capi/capidrv.c 2007-08-24 11:06:46.0 -0700
 @@ -1855,6 +1855,9 @@ static int if_sendbuf(int id, int channe
   return 0;
   }
   datahandle = nccip-datahandle;
 +
 + /* This won't work on 64 bit! */
 + BUILD_BUG_ON(sizeof(skb-data)  sizeof(u32));
   capi_fill_DATA_B3_REQ(sendcmsg, global.ap.applid, card-msgid++,
 nccip-ncci,  /* adr */
 (u32) skb-data,  /* Data */


NACK.

It is not a BUG.

This is OK, since this field must have a value and on 32 it has the correct
one) On 64 bit this field is ignored (but also need a value, random data is
bad as well).

-
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: drivers/infiniband/mlx/mad.c misplaced ;

2007-08-16 Thread Karsten Keil
On Thu, Aug 16, 2007 at 01:22:04PM +0300, Ilpo Järvinen wrote:
 
 ...I guess those guys hunting for broken busyloops in the other thread 
 could also benefit from similar searching commands introduced in this 
 thread... ...Ccing Satyam to caught their attention too.
 
 
 ./drivers/isdn/hisax/hfc_pci.c
 125:  if (Read_hfc(cs, HFCPCI_INT_S1)) ;
 155:  if (Read_hfc(cs, HFCPCI_INT_S1)) ;
 1483: if (Read_hfc(cs, HFCPCI_INT_S1)) ;
 --
 ./drivers/isdn/hisax/hfc_sx.c
 377:  if (Read_hfc(cs, HFCSX_INT_S1)) ;
 407:  if (Read_hfc(cs, HFCSX_INT_S2)) ;
 1246: if (Read_hfc(cs, HFCSX_INT_S1)) ;
 --

These are workaround to not get compiler warnings about ignored return
values I got some time ago under some architecture.


-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 
16746 (AG Nuernberg)
-
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/9] pcmcia: remove prod_id indirection

2006-12-05 Thread Karsten Keil
On Mon, Dec 04, 2006 at 09:12:51PM -0500, Dominik Brodowski wrote:
 From: Dominik Brodowski [EMAIL PROTECTED]
 Date: Sun, 4 Jun 2006 18:06:13 +0200
 Subject: [PATCH] pcmcia: remove prod_id indirection
 
 As we read out the product information strings (VERS_1) from the PCMCIA device
 in the PCMCIA core, and device drivers can access those reliably in struct
 pcmcia_device's fields prod_id[], remove additional product information string
 detection logic from PCMCIA device drivers.
 
 Signed-off-by: Dominik Brodowski [EMAIL PROTECTED]
 ---
  drivers/isdn/hardware/avm/avm_cs.c |   14 +++---
  drivers/isdn/hisax/avma1_cs.c  |   14 +++---

Acked-by: Karsten Keil [EMAIL PROTECTED]
for ISDN.

  drivers/net/pcmcia/3c574_cs.c  |9 +++--
  drivers/net/pcmcia/smc91c92_cs.c   |   27 ---
  drivers/net/pcmcia/xirc2ps_cs.c|   19 ---
  drivers/net/wireless/ray_cs.c  |   15 +--
  drivers/telephony/ixj_pcmcia.c |   30 ++
  7 files changed, 40 insertions(+), 88 deletions(-)
 
 diff --git a/drivers/isdn/hardware/avm/avm_cs.c 
 b/drivers/isdn/hardware/avm/avm_cs.c
 index 7bbfd85..db3755b 100644
 --- a/drivers/isdn/hardware/avm/avm_cs.c
 +++ b/drivers/isdn/hardware/avm/avm_cs.c
 @@ -217,18 +217,10 @@ static int avmcs_config(struct pcmcia_de
  }
  
  do {
 -
 - tuple.Attributes = 0;
 - tuple.TupleData = buf;
 - tuple.TupleDataMax = 254;
 - tuple.TupleOffset = 0;
 - tuple.DesiredTuple = CISTPL_VERS_1;
 -
   devname[0] = 0;
 - if( !first_tuple(link, tuple, parse)  parse.version_1.ns  1 ) {
 - strlcpy(devname,parse.version_1.str + parse.version_1.ofs[1], 
 - sizeof(devname));
 - }
 + if (link-prod_id[1])
 + strlcpy(devname, link-prod_id[1], sizeof(devname));
 +
   /*
   * find IO port
   */
 diff --git a/drivers/isdn/hisax/avma1_cs.c b/drivers/isdn/hisax/avma1_cs.c
 index ac28e32..40c9b02 100644
 --- a/drivers/isdn/hisax/avma1_cs.c
 +++ b/drivers/isdn/hisax/avma1_cs.c
 @@ -239,18 +239,10 @@ static int avma1cs_config(struct pcmcia_
  }
  
  do {
 -
 - tuple.Attributes = 0;
 - tuple.TupleData = buf;
 - tuple.TupleDataMax = 254;
 - tuple.TupleOffset = 0;
 - tuple.DesiredTuple = CISTPL_VERS_1;
 -
   devname[0] = 0;
 - if( !first_tuple(link, tuple, parse)  parse.version_1.ns  1 ) {
 - strlcpy(devname,parse.version_1.str + parse.version_1.ofs[1], 
 - sizeof(devname));
 - }
 + if (link-prod_id[1])
 + strlcpy(devname, link-prod_id[1], sizeof(devname));
 +
   /*
   * find IO port
   */
 diff --git a/drivers/net/pcmcia/3c574_cs.c b/drivers/net/pcmcia/3c574_cs.c
 index 0460099..420f70b 100644
 --- a/drivers/net/pcmcia/3c574_cs.c
 +++ b/drivers/net/pcmcia/3c574_cs.c
 @@ -397,12 +397,9 @@ static int tc574_config(struct pcmcia_de
   goto failed;
   }
   }
 - tuple.DesiredTuple = CISTPL_VERS_1;
 - if (pcmcia_get_first_tuple(link, tuple) == CS_SUCCESS 
 - pcmcia_get_tuple_data(link, tuple) == CS_SUCCESS 
 - pcmcia_parse_tuple(link, tuple, parse) == CS_SUCCESS) {
 - cardname = parse.version_1.str + parse.version_1.ofs[1];
 - } else
 + if (link-prod_id[1])
 + cardname = link-prod_id[1];
 + else
   cardname = 3Com 3c574;
  
   {
 diff --git a/drivers/net/pcmcia/smc91c92_cs.c 
 b/drivers/net/pcmcia/smc91c92_cs.c
 index ae024bf..bf40848 100644
 --- a/drivers/net/pcmcia/smc91c92_cs.c
 +++ b/drivers/net/pcmcia/smc91c92_cs.c
 @@ -560,16 +560,8 @@ static int mhz_setup(struct pcmcia_devic
  
  /* Read the station address from the CIS.  It is stored as the last
 (fourth) string in the Version 1 Version/ID tuple. */
 -tuple-DesiredTuple = CISTPL_VERS_1;
 -if (first_tuple(link, tuple, parse) != CS_SUCCESS) {
 - rc = -1;
 - goto free_cfg_mem;
 -}
 -/* Ugh -- the EM1144 card has two VERS_1 tuples!?! */
 -if (next_tuple(link, tuple, parse) != CS_SUCCESS)
 - first_tuple(link, tuple, parse);
 -if (parse-version_1.ns  3) {
 - station_addr = parse-version_1.str + parse-version_1.ofs[3];
 +if (link-prod_id[3]) {
 + station_addr = link-prod_id[3];
   if (cvt_ascii_address(dev, station_addr) == 0) {
   rc = 0;
   goto free_cfg_mem;
 @@ -744,15 +736,12 @@ static int smc_setup(struct pcmcia_devic
   }
  }
  /* Try the third string in the Version 1 Version/ID tuple. */
 -tuple-DesiredTuple = CISTPL_VERS_1;
 -if (first_tuple(link, tuple, parse) != CS_SUCCESS) {
 - rc = -1;
 - goto free_cfg_mem;
 -}
 -station_addr = parse-version_1.str + parse-version_1.ofs[2];
 -if (cvt_ascii_address(dev, station_addr) == 0) {
 - rc = 0;
 - goto free_cfg_mem;
 +if (link-prod_id[2]) {
 + station_addr = link

Re: [PATCH 5/9] pcmcia: conf.ConfigBase and conf.Present consolidation

2006-12-05 Thread Karsten Keil
On Mon, Dec 04, 2006 at 09:17:16PM -0500, Dominik Brodowski wrote:
 From: Dominik Brodowski [EMAIL PROTECTED]
 Date: Wed, 25 Oct 2006 21:49:27 -0400
 Subject: [PATCH] pcmcia: conf.ConfigBase and conf.Present consolidation
 
 struct pcmcia_device *p_dev-conf.ConfigBase and .Present are set in almost
 all PCMICA driver right at the beginning, using the same calls but slightly
 different implementations. Unfiy this in the PCMCIA core.
 
 Includes a small bugfix (drivers/net/pcmcia/xirc2ps_cs.c: remove unused
 label) from and Signed-off-by Adrian Bunk [EMAIL PROTECTED]
 
 Signed-off-by: Dominik Brodowski [EMAIL PROTECTED]
 ---
  drivers/ata/pata_pcmcia.c   |7 -
  drivers/bluetooth/bluecard_cs.c |   38 
 +--
  drivers/bluetooth/bt3c_cs.c |   20 +---
  drivers/bluetooth/btuart_cs.c   |   20 +---
  drivers/bluetooth/dtl1_cs.c |   20 +---
  drivers/char/pcmcia/cm4000_cs.c |   20 
  drivers/char/pcmcia/cm4040_cs.c |   20 
  drivers/char/pcmcia/synclink_cs.c   |7 -
  drivers/ide/legacy/ide-cs.c |6 -


  drivers/isdn/hardware/avm/avm_cs.c  |   22 --
  drivers/isdn/hisax/avma1_cs.c   |   22 --
  drivers/isdn/hisax/elsa_cs.c|   17 -
  drivers/isdn/hisax/sedlbauer_cs.c   |   10 
  drivers/isdn/hisax/teles_cs.c   |   17 -

Acked-by: Karsten Keil [EMAIL PROTECTED]
for ISDN.

...

-- 
Karsten Keil
SuSE Labs
ISDN development
-
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


[Resend][RFC/PATCH] fix deadlock on high loads in bond_alb_monitor()

2006-09-28 Thread Karsten Keil
In bond_alb_monitor the bond-curr_slave_lock write lock is taken
and then dev_set_promiscuity maybe called which can take some time,
depending on the network HW. If a network IRQ for this card come in
the softirq handler maybe try to deliver more packets which end up in
a request to the read lock of bond-curr_slave_lock - deadlock.
This issue was found by a test lab during network stress tests, this patch
disable the softirq handler for this case and solved the issue.

Signed-off-by: Karsten Keil [EMAIL PROTECTED]


---

 drivers/net/bonding/bond_alb.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

921ada40bf96d7dc9c4258821af3d4616fb3cbae
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e83bc82..3292316 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1433,7 +1433,7 @@ void bond_alb_monitor(struct bonding *bo
 * write lock to protect from other code that also
 * sets the promiscuity.
 */
-   write_lock(bond-curr_slave_lock);
+   write_lock_bh(bond-curr_slave_lock);
 
if (bond_info-primary_is_promisc 
(++bond_info-rlb_promisc_timeout_counter = 
RLB_PROMISC_TIMEOUT)) {
@@ -1448,7 +1448,7 @@ void bond_alb_monitor(struct bonding *bo
bond_info-primary_is_promisc = 0;
}
 
-   write_unlock(bond-curr_slave_lock);
+   write_unlock_bh(bond-curr_slave_lock);
 
if (bond_info-rlb_rebalance) {
bond_info-rlb_rebalance = 0;


-- 
Karsten Keil
SuSE Labs
-
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


[RFC/PATCH] fix deadlock on high loads in bond_alb_monitor()

2006-09-07 Thread Karsten Keil
In bond_alb_monitor the bond-curr_slave_lock write lock is taken
and then dev_set_promiscuity maybe called which can take some time,
depending on the network HW. If a network IRQ for this card come in
the softirq handler maybe try to deliver more packets which end up in
a request to the read lock of bond-curr_slave_lock - deadlock.
This issue was found by a test lab during network stress tests, this patch
disable the softirq handler for this case and solved the issue.

Signed-off-by: Karsten Keil [EMAIL PROTECTED]


---

 drivers/net/bonding/bond_alb.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

921ada40bf96d7dc9c4258821af3d4616fb3cbae
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e83bc82..3292316 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1433,7 +1433,7 @@ void bond_alb_monitor(struct bonding *bo
 * write lock to protect from other code that also
 * sets the promiscuity.
 */
-   write_lock(bond-curr_slave_lock);
+   write_lock_bh(bond-curr_slave_lock);
 
if (bond_info-primary_is_promisc 
(++bond_info-rlb_promisc_timeout_counter = 
RLB_PROMISC_TIMEOUT)) {
@@ -1448,7 +1448,7 @@ void bond_alb_monitor(struct bonding *bo
bond_info-primary_is_promisc = 0;
}
 
-   write_unlock(bond-curr_slave_lock);
+   write_unlock_bh(bond-curr_slave_lock);
 
if (bond_info-rlb_rebalance) {
bond_info-rlb_rebalance = 0;


-- 
Karsten Keil
SuSE Labs
ISDN development
-
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] ethtool always report port is TP on tg3

2006-05-12 Thread Karsten Keil

Even with fiber cards ethtool reports that the connected port is TP,
the patch fix this.

---

 drivers/net/tg3.c |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

5ed8e79c778ee803e44a325a1e15c0cb3f52d0ff
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index beeb612..0b5bc93 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -7653,21 +7653,23 @@ static int tg3_get_settings(struct net_d
cmd-supported |= (SUPPORTED_1000baseT_Half |
   SUPPORTED_1000baseT_Full);
 
-   if (!(tp-tg3_flags2  TG3_FLG2_ANY_SERDES))
+   if (!(tp-tg3_flags2  TG3_FLG2_ANY_SERDES)) {
cmd-supported |= (SUPPORTED_100baseT_Half |
  SUPPORTED_100baseT_Full |
  SUPPORTED_10baseT_Half |
  SUPPORTED_10baseT_Full |
  SUPPORTED_MII);
-   else
+   cmd-port = PORT_TP;
+   } else {
cmd-supported |= SUPPORTED_FIBRE;
+   cmd-port = PORT_FIBRE;
+   }
   
cmd-advertising = tp-link_config.advertising;
if (netif_running(dev)) {
cmd-speed = tp-link_config.active_speed;
cmd-duplex = tp-link_config.active_duplex;
}
-   cmd-port = 0;
cmd-phy_address = PHY_ADDR;
cmd-transceiver = 0;
cmd-autoneg = tp-link_config.autoneg;
-- 
Karsten Keil
SuSE Labs
ISDN development
-
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] ethtool always report port is TP on tg3

2006-05-12 Thread Karsten Keil

On Fri, May 12, 2006 at 07:59:54AM -0700, Michael Chan wrote:

 ACK. Thanks. Please add sign-off line and send to DaveM.

This time with Signed-off line.


Even with fiber cards ethtool reports that the connected port is TP,
the patch fix this.

Signed-off-by: Karsten Keil [EMAIL PROTECTED]
---

 drivers/net/tg3.c |8 +---
 1 files changed, 5 insertions(+), 3 deletions(-)

5ed8e79c778ee803e44a325a1e15c0cb3f52d0ff
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index beeb612..0b5bc93 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -7653,21 +7653,23 @@ static int tg3_get_settings(struct net_d
cmd-supported |= (SUPPORTED_1000baseT_Half |
   SUPPORTED_1000baseT_Full);
 
-   if (!(tp-tg3_flags2  TG3_FLG2_ANY_SERDES))
+   if (!(tp-tg3_flags2  TG3_FLG2_ANY_SERDES)) {
cmd-supported |= (SUPPORTED_100baseT_Half |
  SUPPORTED_100baseT_Full |
  SUPPORTED_10baseT_Half |
  SUPPORTED_10baseT_Full |
  SUPPORTED_MII);
-   else
+   cmd-port = PORT_TP;
+   } else {
cmd-supported |= SUPPORTED_FIBRE;
+   cmd-port = PORT_FIBRE;
+   }
   
cmd-advertising = tp-link_config.advertising;
if (netif_running(dev)) {
cmd-speed = tp-link_config.active_speed;
cmd-duplex = tp-link_config.active_duplex;
}
-   cmd-port = 0;
cmd-phy_address = PHY_ADDR;
cmd-transceiver = 0;
cmd-autoneg = tp-link_config.autoneg;

-- 
Karsten Keil
SuSE Labs
ISDN development
-
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


remove Davicom DM9102 PCI id from tulip_core.c ?

2005-11-16 Thread Karsten Keil
Hello Jeff,

I got some  user reports that they have problems with a
Davicom DM9102 NIC and the tulip driver.

PCI ID: 1282:9102 (rev 20)

Symptoms:
NIC gets address via DHCP, ping works but real traffic stalls
after some packets.

Switching to the dmfe driver made it working for them.

So I would suggest to remove this ID from the generic driver,
since it seems that dmfe is the better choice for this NIC.

What do you think ?

--- linux-2.6.15-rc1.4/drivers/net/tulip/tulip_core.c.org   2005-11-16 
14:25:57.0 +0100
+++ linux-2.6.15-rc1.4/drivers/net/tulip/tulip_core.c   2005-11-16 
14:35:33.0 +0100
@@ -224,7 +224,6 @@
{ 0x11F6, 0x9881, PCI_ANY_ID, PCI_ANY_ID, 0, 0, COMPEX9881 },
{ 0x8086, 0x0039, PCI_ANY_ID, PCI_ANY_ID, 0, 0, I21145 },
{ 0x1282, 0x9100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, DM910X },
-   { 0x1282, 0x9102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, DM910X },
{ 0x1113, 0x1216, PCI_ANY_ID, PCI_ANY_ID, 0, 0, COMET },
{ 0x1113, 0x1217, PCI_ANY_ID, PCI_ANY_ID, 0, 0, MX98715 },
{ 0x1113, 0x9511, PCI_ANY_ID, PCI_ANY_ID, 0, 0, COMET },

-- 
Karsten Keil
SuSE Labs
ISDN development
-
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