* [PATCH 1/2][IPV4] Consolidate the ip cork destruction in ip_output.c
From: Pavel Emelyanov @ 2007-11-02 16:04 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
The ip_push_pending_frames and ip_flush_pending_frames do the
same things to flush the sock's cork. Move this into a separate
function and save ~80 bytes from the .text
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index e5f7dc2..fd99fbd 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1183,6 +1183,17 @@ error:
return err;
}
+static void ip_cork_release(struct inet_sock *inet)
+{
+ inet->cork.flags &= ~IPCORK_OPT;
+ kfree(inet->cork.opt);
+ inet->cork.opt = NULL;
+ if (inet->cork.rt) {
+ ip_rt_put(inet->cork.rt);
+ inet->cork.rt = NULL;
+ }
+}
+
/*
* Combined all pending IP fragments on the socket as one IP datagram
* and push them out.
@@ -1276,13 +1287,7 @@ int ip_push_pending_frames(struct sock *sk)
}
out:
- inet->cork.flags &= ~IPCORK_OPT;
- kfree(inet->cork.opt);
- inet->cork.opt = NULL;
- if (inet->cork.rt) {
- ip_rt_put(inet->cork.rt);
- inet->cork.rt = NULL;
- }
+ ip_cork_release(inet);
return err;
error:
@@ -1295,19 +1300,12 @@ error:
*/
void ip_flush_pending_frames(struct sock *sk)
{
- struct inet_sock *inet = inet_sk(sk);
struct sk_buff *skb;
while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL)
kfree_skb(skb);
- inet->cork.flags &= ~IPCORK_OPT;
- kfree(inet->cork.opt);
- inet->cork.opt = NULL;
- if (inet->cork.rt) {
- ip_rt_put(inet->cork.rt);
- inet->cork.rt = NULL;
- }
+ ip_cork_release(inet_sk(sk));
}
^ permalink raw reply related
* [PATCH 2/2][IPV6] Consolidate the ip cork destruction in ip6_output.c
From: Pavel Emelyanov @ 2007-11-02 16:05 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
The ip6_push_pending_frames and ip6_flush_pending_frames do the
same things to flush the sock's cork. Move this into a separate
function and save ~100 bytes from the .text
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 653fc0a..f4e9e73 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1339,6 +1339,19 @@ error:
return err;
}
+static void ip6_cork_release(struct inet_sock *inet, struct ipv6_pinfo *np)
+{
+ inet->cork.flags &= ~IPCORK_OPT;
+ kfree(np->cork.opt);
+ np->cork.opt = NULL;
+ if (np->cork.rt) {
+ dst_release(&np->cork.rt->u.dst);
+ np->cork.rt = NULL;
+ inet->cork.flags &= ~IPCORK_ALLFRAG;
+ }
+ memset(&inet->cork.fl, 0, sizeof(inet->cork.fl));
+}
+
int ip6_push_pending_frames(struct sock *sk)
{
struct sk_buff *skb, *tmp_skb;
@@ -1415,15 +1428,7 @@ int ip6_push_pending_frames(struct sock *sk)
}
out:
- inet->cork.flags &= ~IPCORK_OPT;
- kfree(np->cork.opt);
- np->cork.opt = NULL;
- if (np->cork.rt) {
- dst_release(&np->cork.rt->u.dst);
- np->cork.rt = NULL;
- inet->cork.flags &= ~IPCORK_ALLFRAG;
- }
- memset(&inet->cork.fl, 0, sizeof(inet->cork.fl));
+ ip6_cork_release(inet, np);
return err;
error:
goto out;
@@ -1431,8 +1436,6 @@ error:
void ip6_flush_pending_frames(struct sock *sk)
{
- struct inet_sock *inet = inet_sk(sk);
- struct ipv6_pinfo *np = inet6_sk(sk);
struct sk_buff *skb;
while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL) {
@@ -1442,14 +1445,5 @@ void ip6_flush_pending_frames(struct sock *sk)
kfree_skb(skb);
}
- inet->cork.flags &= ~IPCORK_OPT;
-
- kfree(np->cork.opt);
- np->cork.opt = NULL;
- if (np->cork.rt) {
- dst_release(&np->cork.rt->u.dst);
- np->cork.rt = NULL;
- inet->cork.flags &= ~IPCORK_ALLFRAG;
- }
- memset(&inet->cork.fl, 0, sizeof(inet->cork.fl));
+ ip6_cork_release(inet_sk(sk), inet6_sk(sk));
}
--
1.5.3.4
^ permalink raw reply related
* Re: [PATCH] Fix e100 on systems that have cache incoherent DMA
From: Kok, Auke @ 2007-11-02 16:05 UTC (permalink / raw)
To: David Acker
Cc: Auke Kok, John Ronciak, Jesse Brandeburg, Jeff Kirsher,
Milton Miller, Jeff Garzik, netdev, e1000-devel, Scott Feldman
In-Reply-To: <20071102132703.1652446C128@localhost>
David Acker wrote:
> On the systems that have cache incoherent DMA, including ARM, there is a
> race condition between software allocating a new receive buffer and hardware
> writing into a buffer. The two race on touching the last Receive Frame
> Descriptor (RFD). It has its el-bit set and its next link equal to 0.
> When hardware encounters this buffer it attempts to write data to it and
> then update Status Word bits and Actual Count in the RFD. At the same time
> software may try to clear the el-bit and set the link address to a new buffer.
>
> Since the entire RFD is once cache-line, the two write operations can collide.
> This can lead to the receive unit stalling or interpreting random memory as
> its receive area.
>
> The fix is to set the el-bit on and the size to 0 on the next to last buffer
> in the chain. When the hardware encounters this buffer it stops and does not
> write to it at all. The hardware issues an RNR interrupt with the receive
> unit in the No Resources state. Software can write to the tail of the list
> because it knows hardware will stop on the previous descriptor that was
> marked as the end of list.
>
> Once it has a new next to last buffer prepared, it can clear the el-bit and
> set the size on the previous one. The race on this buffer is safe since
> the link already points to a valid next buffer and the software can handle
> the race setting the size (assuming aligned 16 bit writes are atomic with
> respect to the DMA read). If the hardware sees the el-bit cleared without
> the size set, it will move on to the next buffer and skip this one. If it
> sees the size set but the el-bit still set, it will complete that buffer
> and then RNR interrupt and wait.
>
>
> This is a patch for 2.6.24-rc1.
>
> Signed-off-by: David Acker <dacker@roinet.com>
>
> ---
>
> This version is based on the simpler patch I did in May. The algorithm I tried
> after that never worked correctly under load. It would hang the RU and the
> transmit unit sometimes and if the card was restarted it would often crash the
> system with memory corruption. This patch was tested on my embedded system
> using pktgen. I had it sending while a PC sent at it. I also ran it as
> wireless access point with a 12-hour bidirectional 20 mbps UDP going between an
> ethernet host on the e100 and a wireless client.
looks much simpler to me too, which I like.
It's good to see something coming from you! I'm going to make sure this gets on
the test bench today and will keep you posted on the progress. We'll take a few
days to make sure that this doesn't break early.
Thanks!!!
Auke
>
> --- linux-2.6.24-rc1/drivers/net/e100.c.orig 2007-11-01 11:42:35.000000000 -0400
> +++ linux-2.6.24-rc1/drivers/net/e100.c 2007-11-02 09:09:47.000000000 -0400
> @@ -106,6 +106,13 @@
> * the RFD, the RFD must be dma_sync'ed to maintain a consistent
> * view from software and hardware.
> *
> + * In order to keep updates to the RFD link field from colliding with
> + * hardware writes to mark packets complete, we use the feature that
> + * hardware will not write to a size 0 descriptor and mark the previous
> + * packet as end-of-list (EL). After updating the link, we remove EL
> + * and only then restore the size such that hardware may use the
> + * previous-to-end RFD.
> + *
> * Under typical operation, the receive unit (RU) is start once,
> * and the controller happily fills RFDs as frames arrive. If
> * replacement RFDs cannot be allocated, or the RU goes non-active,
> @@ -281,14 +288,15 @@ struct csr {
> };
>
> enum scb_status {
> + rus_no_res = 0x08,
> rus_ready = 0x10,
> rus_mask = 0x3C,
> };
>
> enum ru_state {
> - RU_SUSPENDED = 0,
> - RU_RUNNING = 1,
> - RU_UNINITIALIZED = -1,
> + ru_stopped = 0,
> + ru_running = 1,
> + ru_uninitialized = -1,
> };
>
> enum scb_stat_ack {
> @@ -952,7 +960,7 @@ static void e100_get_defaults(struct nic
> ((nic->mac >= mac_82558_D101_A4) ? cb_cid : cb_i));
>
> /* Template for a freshly allocated RFD */
> - nic->blank_rfd.command = cpu_to_le16(cb_el);
> + nic->blank_rfd.command = 0;
> nic->blank_rfd.rbd = 0xFFFFFFFF;
> nic->blank_rfd.size = cpu_to_le16(VLAN_ETH_FRAME_LEN);
>
> @@ -1759,7 +1767,7 @@ static int e100_alloc_cbs(struct nic *ni
> static inline void e100_start_receiver(struct nic *nic, struct rx *rx)
> {
> if(!nic->rxs) return;
> - if(RU_SUSPENDED != nic->ru_running) return;
> + if (ru_stopped != nic->ru_running) return;
>
> /* handle init time starts */
> if(!rx) rx = nic->rxs;
> @@ -1767,7 +1775,7 @@ static inline void e100_start_receiver(s
> /* (Re)start RU if suspended or idle and RFA is non-NULL */
> if(rx->skb) {
> e100_exec_cmd(nic, ruc_start, rx->dma_addr);
> - nic->ru_running = RU_RUNNING;
> + nic->ru_running = ru_running;
> }
> }
>
> @@ -1791,15 +1799,12 @@ static int e100_rx_alloc_skb(struct nic
> }
>
> /* Link the RFD to end of RFA by linking previous RFD to
> - * this one, and clearing EL bit of previous. */
> + * this one. We are safe to touch the previous RFD because
> + * it is protected by the before last buffer's el bit being set */
> if(rx->prev->skb) {
> struct rfd *prev_rfd = (struct rfd *)rx->prev->skb->data;
> put_unaligned(cpu_to_le32(rx->dma_addr),
> (u32 *)&prev_rfd->link);
> - wmb();
> - prev_rfd->command &= ~cpu_to_le16(cb_el);
> - pci_dma_sync_single_for_device(nic->pdev, rx->prev->dma_addr,
> - sizeof(struct rfd), PCI_DMA_TODEVICE);
> }
>
> return 0;
> @@ -1824,8 +1829,20 @@ static int e100_rx_indicate(struct nic *
> DPRINTK(RX_STATUS, DEBUG, "status=0x%04X\n", rfd_status);
>
> /* If data isn't ready, nothing to indicate */
> - if(unlikely(!(rfd_status & cb_complete)))
> + if (unlikely(!(rfd_status & cb_complete))) {
> + /* If the next buffer has the el bit, but we think the receiver
> + * is still running, check to see if it really stopped while
> + * we had interrupts off.
> + * This allows for a fast restart without re-enabling
> + * interrupts */
> + if ((le16_to_cpu(rfd->command) & cb_el) &&
> + (ru_running == nic->ru_running)) {
> +
> + if (readb(&nic->csr->scb.status) & rus_no_res)
> + nic->ru_running = ru_stopped;
> + }
> return -ENODATA;
> + }
>
> /* Get actual data size */
> actual_size = le16_to_cpu(rfd->actual_size) & 0x3FFF;
> @@ -1836,9 +1853,18 @@ static int e100_rx_indicate(struct nic *
> pci_unmap_single(nic->pdev, rx->dma_addr,
> RFD_BUF_LEN, PCI_DMA_FROMDEVICE);
>
> - /* this allows for a fast restart without re-enabling interrupts */
> - if(le16_to_cpu(rfd->command) & cb_el)
> - nic->ru_running = RU_SUSPENDED;
> + /* If this buffer has the el bit, but we think the receiver
> + * is still running, check to see if it really stopped while
> + * we had interrupts off.
> + * This allows for a fast restart without re-enabling interrupts.
> + * This can happen when the RU sees the size change but also sees
> + * the el bit set. */
> + if ((le16_to_cpu(rfd->command) & cb_el) &&
> + (ru_running == nic->ru_running)) {
> +
> + if (readb(&nic->csr->scb.status) & rus_no_res)
> + nic->ru_running = ru_stopped;
> + }
>
> /* Pull off the RFD and put the actual data (minus eth hdr) */
> skb_reserve(skb, sizeof(struct rfd));
> @@ -1870,31 +1896,30 @@ static void e100_rx_clean(struct nic *ni
> unsigned int work_to_do)
> {
> struct rx *rx;
> - int restart_required = 0;
> - struct rx *rx_to_start = NULL;
> -
> - /* are we already rnr? then pay attention!!! this ensures that
> - * the state machine progression never allows a start with a
> - * partially cleaned list, avoiding a race between hardware
> - * and rx_to_clean when in NAPI mode */
> - if(RU_SUSPENDED == nic->ru_running)
> - restart_required = 1;
> + int restart_required = 0, err = 0;
> + struct rx *old_before_last_rx, *new_before_last_rx;
> + struct rfd *old_before_last_rfd, *new_before_last_rfd;
>
> /* Indicate newly arrived packets */
> for(rx = nic->rx_to_clean; rx->skb; rx = nic->rx_to_clean = rx->next) {
> - int err = e100_rx_indicate(nic, rx, work_done, work_to_do);
> - if(-EAGAIN == err) {
> - /* hit quota so have more work to do, restart once
> - * cleanup is complete */
> - restart_required = 0;
> + err = e100_rx_indicate(nic, rx, work_done, work_to_do);
> + /* Hit quota or no more to clean */
> + if (-EAGAIN == err || -ENODATA == err)
> break;
> - } else if(-ENODATA == err)
> - break; /* No more to clean */
> }
>
> - /* save our starting point as the place we'll restart the receiver */
> - if(restart_required)
> - rx_to_start = nic->rx_to_clean;
> +
> + /* On EAGAIN, hit quota so have more work to do, restart once
> + * cleanup is complete.
> + * Else, are we already rnr? then pay attention!!! this ensures that
> + * the state machine progression never allows a start with a
> + * partially cleaned list, avoiding a race between hardware
> + * and rx_to_clean when in NAPI mode */
> + if (-EAGAIN != err && ru_stopped == nic->ru_running)
> + restart_required = 1;
> +
> + old_before_last_rx = nic->rx_to_use->prev->prev;
> + old_before_last_rfd = (struct rfd *)old_before_last_rx->skb->data;
>
> /* Alloc new skbs to refill list */
> for(rx = nic->rx_to_use; !rx->skb; rx = nic->rx_to_use = rx->next) {
> @@ -1902,10 +1927,42 @@ static void e100_rx_clean(struct nic *ni
> break; /* Better luck next time (see watchdog) */
> }
>
> + new_before_last_rx = nic->rx_to_use->prev->prev;
> + if (new_before_last_rx != old_before_last_rx) {
> + /* Set the el-bit on the buffer that is before the last buffer.
> + * This lets us update the next pointer on the last buffer
> + * without worrying about hardware touching it.
> + * We set the size to 0 to prevent hardware from touching this
> + * buffer.
> + * When the hardware hits the before last buffer with el-bit
> + * and size of 0, it will RNR interrupt, the RUS will go into
> + * the No Resources state. It will not complete nor write to
> + * this buffer. */
> + new_before_last_rfd =
> + (struct rfd *)new_before_last_rx->skb->data;
> + new_before_last_rfd->size = 0;
> + new_before_last_rfd->command |= cpu_to_le16(cb_el);
> + pci_dma_sync_single_for_device(nic->pdev,
> + new_before_last_rx->dma_addr, sizeof(struct rfd),
> + PCI_DMA_TODEVICE);
> +
> + /* Now that we have a new stopping point, we can clear the old
> + * stopping point. We must sync twice to get the proper
> + * ordering on the hardware side of things. */
> + old_before_last_rfd->command &= ~cpu_to_le16(cb_el);
> + pci_dma_sync_single_for_device(nic->pdev,
> + old_before_last_rx->dma_addr, sizeof(struct rfd),
> + PCI_DMA_TODEVICE);
> + old_before_last_rfd->size = cpu_to_le16(VLAN_ETH_FRAME_LEN);
> + pci_dma_sync_single_for_device(nic->pdev,
> + old_before_last_rx->dma_addr, sizeof(struct rfd),
> + PCI_DMA_TODEVICE);
> + }
> +
> if(restart_required) {
> // ack the rnr?
> writeb(stat_ack_rnr, &nic->csr->scb.stat_ack);
> - e100_start_receiver(nic, rx_to_start);
> + e100_start_receiver(nic, nic->rx_to_clean);
> if(work_done)
> (*work_done)++;
> }
> @@ -1916,7 +1973,7 @@ static void e100_rx_clean_list(struct ni
> struct rx *rx;
> unsigned int i, count = nic->params.rfds.count;
>
> - nic->ru_running = RU_UNINITIALIZED;
> + nic->ru_running = ru_uninitialized;
>
> if(nic->rxs) {
> for(rx = nic->rxs, i = 0; i < count; rx++, i++) {
> @@ -1937,9 +1994,10 @@ static int e100_rx_alloc_list(struct nic
> {
> struct rx *rx;
> unsigned int i, count = nic->params.rfds.count;
> + struct rfd *before_last;
>
> nic->rx_to_use = nic->rx_to_clean = NULL;
> - nic->ru_running = RU_UNINITIALIZED;
> + nic->ru_running = ru_uninitialized;
>
> if(!(nic->rxs = kcalloc(count, sizeof(struct rx), GFP_ATOMIC)))
> return -ENOMEM;
> @@ -1952,9 +2010,22 @@ static int e100_rx_alloc_list(struct nic
> return -ENOMEM;
> }
> }
> + /* Set the el-bit on the buffer that is before the last buffer.
> + * This lets us update the next pointer on the last buffer without
> + * worrying about hardware touching it.
> + * We set the size to 0 to prevent hardware from touching this buffer.
> + * When the hardware hits the before last buffer with el-bit and size
> + * of 0, it will RNR interrupt, the RU will go into the No Resources
> + * state. It will not complete nor write to this buffer. */
> + rx = nic->rxs->prev->prev;
> + before_last = (struct rfd *)rx->skb->data;
> + before_last->command |= cpu_to_le16(cb_el);
> + before_last->size = 0;
> + pci_dma_sync_single_for_device(nic->pdev, rx->dma_addr,
> + sizeof(struct rfd), PCI_DMA_TODEVICE);
>
> nic->rx_to_use = nic->rx_to_clean = nic->rxs;
> - nic->ru_running = RU_SUSPENDED;
> + nic->ru_running = ru_stopped;
>
> return 0;
> }
> @@ -1976,7 +2047,7 @@ static irqreturn_t e100_intr(int irq, vo
>
> /* We hit Receive No Resource (RNR); restart RU after cleaning */
> if(stat_ack & stat_ack_rnr)
> - nic->ru_running = RU_SUSPENDED;
> + nic->ru_running = ru_stopped;
>
> if(likely(netif_rx_schedule_prep(netdev, &nic->napi))) {
> e100_disable_irq(nic);
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Fix e100 on systems that have cache incoherent DMA
From: Jeff Garzik @ 2007-11-02 16:11 UTC (permalink / raw)
To: Kok, Auke
Cc: David Acker, John Ronciak, Jesse Brandeburg, Jeff Kirsher,
Milton Miller, netdev, e1000-devel, Scott Feldman
In-Reply-To: <472B4AD0.4020504@intel.com>
Kok, Auke wrote:
> David Acker wrote:
>> On the systems that have cache incoherent DMA, including ARM, there is a
>> race condition between software allocating a new receive buffer and hardware
>> writing into a buffer. The two race on touching the last Receive Frame
>> Descriptor (RFD). It has its el-bit set and its next link equal to 0.
>> When hardware encounters this buffer it attempts to write data to it and
>> then update Status Word bits and Actual Count in the RFD. At the same time
>> software may try to clear the el-bit and set the link address to a new buffer.
>>
>> Since the entire RFD is once cache-line, the two write operations can collide.
>> This can lead to the receive unit stalling or interpreting random memory as
>> its receive area.
>>
>> The fix is to set the el-bit on and the size to 0 on the next to last buffer
>> in the chain. When the hardware encounters this buffer it stops and does not
>> write to it at all. The hardware issues an RNR interrupt with the receive
>> unit in the No Resources state. Software can write to the tail of the list
>> because it knows hardware will stop on the previous descriptor that was
>> marked as the end of list.
>>
>> Once it has a new next to last buffer prepared, it can clear the el-bit and
>> set the size on the previous one. The race on this buffer is safe since
>> the link already points to a valid next buffer and the software can handle
>> the race setting the size (assuming aligned 16 bit writes are atomic with
>> respect to the DMA read). If the hardware sees the el-bit cleared without
>> the size set, it will move on to the next buffer and skip this one. If it
>> sees the size set but the el-bit still set, it will complete that buffer
>> and then RNR interrupt and wait.
>>
>>
>> This is a patch for 2.6.24-rc1.
>>
>> Signed-off-by: David Acker <dacker@roinet.com>
>>
>> ---
>>
>> This version is based on the simpler patch I did in May. The algorithm I tried
>> after that never worked correctly under load. It would hang the RU and the
>> transmit unit sometimes and if the card was restarted it would often crash the
>> system with memory corruption. This patch was tested on my embedded system
>> using pktgen. I had it sending while a PC sent at it. I also ran it as
>> wireless access point with a 12-hour bidirectional 20 mbps UDP going between an
>> ethernet host on the e100 and a wireless client.
>
> looks much simpler to me too, which I like.
>
> It's good to see something coming from you! I'm going to make sure this gets on
> the test bench today and will keep you posted on the progress. We'll take a few
> days to make sure that this doesn't break early.
>
> Thanks!!!
Agreed, I _really_ appreciate this effort being kept alive.
Jeff
^ permalink raw reply
* Re: Endianness problem with u32 classifier hash masks
From: Jarek Poplawski @ 2007-11-02 17:31 UTC (permalink / raw)
To: Radu Rendec; +Cc: netdev, jamal
In-Reply-To: <1193939701.2987.82.camel@localhost.localdomain>
Radu Rendec wrote:
> Hi,
>
> While trying to implement u32 hashes in my shaping machine I ran into a
> possible bug in the u32 hash/bucket computing algorithm
> (net/sched/cls_u32.c).
>
> The problem occurs only with hash masks that extend over the octet
> boundary, on little endian machines (where htonl() actually does
> something).
>
> I'm not 100% sure this is a problem with u32 itself, but at least I'm
> sure u32 with the same configuration would behave differently on little
> endian and big endian machines. Detailed description of the problem and
> proposed patch follow.
I think you are right about this different behavior, so it looks like a bug.
And since little endian way is uncontrollable in such a case, your proposal
should be right.
But, since there is a maintainer for this, let's check what is he not payed
for?! (Cc: Jamal Hadi Salim)
Regards,
Jarek P.
>
> Let's say that I would like to use 0x3fc0 as the hash mask. This means 8
> contiguous "1" bits starting at b6. With such a mask, the expected (and
> logical) behavior is to hash any address in, for instance,
> 192.168.0.0/26 in bucket 0, then any address in 192.168.0.64/26 in
> bucket 1, then 192.168.0.128/26 in bucket 2 and so on.
>
> This is exactly what would happen on a big endian machine, but on little
> endian machines, what would actually happen with current implementation
> is 0x3fc0 being reversed (into 0xc03f0000) by htonl() in the userspace
> tool and then applied to 192.168.x.x in the u32 classifier. When
> shifting right by 16 bits (rank of first "1" bit in the reversed mask)
> and applying the divisor mask (0xff for divisor 256), what would
> actually remain is 0x3f applied on the "168" octet of the address.
>
> One could say is this can be easily worked around by taking endianness
> into account in userspace and supplying an appropriate mask (0xfc03)
> that would be turned into contiguous "1" bits when reversed
> (0x03fc0000). But the actual problem is the network address (inside the
> packet) not being converted to host order, but used as a host-order
> value when computing the bucket.
>
> Let's say the network address is written as n31 n30 ... n0, with n0
> being the least significant bit. When used directly (without any
> conversion) on a little endian machine, it becomes
> n7 ... n0 n8 ..n15 etc in the machine's registers. Thus bits n7 and n8
> would no longer be adjacent and 192.168.64.0/26 and 192.168.128.0/26
> would no longer be consecutive.
>
> My approach to this issue was keeping the hash mask in host order and
> converting the octets in the packet to host order before applying the
> mask. This proved to work just fine on my little endian machine, but I'm
> interested in finding out (from you) if this really is an issue with u32
> itself.
>
> My changes to the u32 classifier are attached below as a patch. It was
> made against 2.6.22.9, but applies cleanly on Dave Miller's net-2.6
> tree.
>
> The idea behind my changes is to keep the user space tool intact and
> work everything out in kernel space (because converting the packet
> octets to host order must be done in kernel anyway).
>
> Therefore, hash masks are converted back to host order when a selector
> is configured - in u32_change() - and converted to network order
> (because userspace tools expect to get them in network order from the
> kernel) when a selector is dumped - in u32_dump().
>
> I would like at least to know your opinion about this issue.
>
> Thanks,
>
> Radu Rendec
>
> --- linux-2.6.22.9/net/sched/cls_u32.c.orig 2007-10-30 17:08:03.000000000 +0200
> +++ linux-2.6.22.9/net/sched/cls_u32.c 2007-10-30 17:04:49.000000000 +0200
> @@ -198,7 +198,7 @@
> ht = n->ht_down;
> sel = 0;
> if (ht->divisor)
> - sel = ht->divisor&u32_hash_fold(*(u32*)(ptr+n->sel.hoff), &n->sel,n->fshift);
> + sel = ht->divisor&u32_hash_fold(ntohl(*(u32*)(ptr+n->sel.hoff)), &n->sel,n->fshift);
>
> if (!(n->sel.flags&(TC_U32_VAROFFSET|TC_U32_OFFSET|TC_U32_EAT)))
> goto next_ht;
> @@ -626,6 +626,10 @@
> }
> #endif
>
> + /* userspace tc tool sends us the hmask in network order, but we
> + * need host order, so change it here */
> + s->hmask = ntohl(s->hmask);
> +
> memcpy(&n->sel, s, sizeof(*s) + s->nkeys*sizeof(struct tc_u32_key));
> n->ht_up = ht;
> n->handle = handle;
> @@ -735,9 +739,14 @@
> u32 divisor = ht->divisor+1;
> RTA_PUT(skb, TCA_U32_DIVISOR, 4, &divisor);
> } else {
> + /* get the address where the selector will be put, then
> + * change the hmask after it is put there */
> + struct tc_u32_sel *s =
> + (struct tc_u32_sel *)RTA_DATA(skb_tail_pointer(skb));
> RTA_PUT(skb, TCA_U32_SEL,
> sizeof(n->sel) + n->sel.nkeys*sizeof(struct tc_u32_key),
> &n->sel);
> + s->hmask = htonl(s->hmask);
> if (n->ht_up) {
> u32 htid = n->handle & 0xFFFFF000;
> RTA_PUT(skb, TCA_U32_HASH, 4, &htid);
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [BUG] in inet6_create
From: Roel Kluin @ 2007-11-02 17:51 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: netdev, linux-net
In-Reply-To: <472AEAB0.5090709@openvz.org>
Pavel Emelyanov wrote:
> Roel Kluin wrote:
>> Roel Kluin wrote:
>>> I got this bug recently, I am not sure whether this is related to any previously
>>> reported ones. It was a recently pulled git kernel. Also I have been hacking my
>>> kernel a bit lately, but I think that I haven't got any changes in the currently
>>> running kernel.
>>>
>>> FYI: my network card was not running (module not loaded, and I just started
>>> thunderbird)
>>>
>>> Roel
>>>
>>> More information needed?
>
> Yes, please.
>
> Can you send us the disasm (objdump -dr) of your ipv6 module.
> More precisely - I need the disassembled inet6_create() function to
> figure out where exactly this thing happened.
I was very lucky to still be able to produce this: When the bug hit me, I had just
recompiled a new kernel, however, since I had previously git-pulled, (but not yet
compiled) the old module was not overwritten.
to answer the question in your other mail - whether I hacked this kernel - I am not
100% certain, I am certain, however that I did not touch IPv6 code, and my changes
to net code were very trivial oneliner changes that I have previously posted, and
were generally accepted as fixes.
--
000002f0 <inet6_create>:
2f0: 55 push %ebp
2f1: bd 9f ff ff ff mov $0xffffff9f,%ebp
2f6: 57 push %edi
2f7: 56 push %esi
2f8: 89 ce mov %ecx,%esi
2fa: 53 push %ebx
2fb: 83 ec 20 sub $0x20,%esp
2fe: 3d 00 00 00 00 cmp $0x0,%eax
2ff: R_386_32 init_net
303: 89 54 24 10 mov %edx,0x10(%esp)
307: 74 0a je 313 <inet6_create+0x23>
309: 83 c4 20 add $0x20,%esp
30c: 89 e8 mov %ebp,%eax
30e: 5b pop %ebx
30f: 5e pop %esi
310: 5f pop %edi
311: 5d pop %ebp
312: c3 ret
313: 8b 42 3c mov 0x3c(%edx),%eax
316: 83 e8 02 sub $0x2,%eax
319: 66 83 f8 01 cmp $0x1,%ax
31d: 76 0e jbe 32d <inet6_create+0x3d>
31f: 8b 0d 00 00 00 00 mov 0x0,%ecx
321: R_386_32 inet_ehash_secret
325: 85 c9 test %ecx,%ecx
327: 0f 84 76 02 00 00 je 5a3 <inet6_create+0x2b3>
32d: c7 44 24 18 00 00 00 movl $0x0,0x18(%esp)
334: 00
335: 31 d2 xor %edx,%edx
337: 31 c9 xor %ecx,%ecx
339: b8 00 00 00 00 mov $0x0,%eax
33a: R_386_32 rcu_lock_map
33e: c7 44 24 08 35 03 00 movl $0x335,0x8(%esp)
345: 00
342: R_386_32 .text
346: c7 44 24 04 01 00 00 movl $0x1,0x4(%esp)
34d: 00
34e: c7 04 24 02 00 00 00 movl $0x2,(%esp)
355: e8 fc ff ff ff call 356 <inet6_create+0x66>
356: R_386_PC32 lock_acquire
35a: 8b 44 24 10 mov 0x10(%esp),%eax
35e: 8b 78 3c mov 0x3c(%eax),%edi
361: 0f bf c7 movswl %di,%eax
364: c1 e0 03 shl $0x3,%eax
367: 8b 98 00 00 00 00 mov 0x0(%eax),%ebx
369: R_386_32 .bss
36d: 8d 90 00 00 00 00 lea 0x0(%eax),%edx
36f: R_386_32 .bss
373: 89 5c 24 1c mov %ebx,0x1c(%esp)
377: 8b 44 24 1c mov 0x1c(%esp),%eax
37b: 8b 00 mov (%eax),%eax
37d: 8d 44 20 00 lea 0x0(%eax),%eax
381: 39 d3 cmp %edx,%ebx
383: bd a2 ff ff ff mov $0xffffffa2,%ebp
388: 75 3a jne 3c4 <inet6_create+0xd4>
38a: e9 23 02 00 00 jmp 5b2 <inet6_create+0x2c2>
38f: 90 nop
390: 85 f6 test %esi,%esi
392: 0f 84 5d 02 00 00 je 5f5 <inet6_create+0x305>
398: 66 85 c0 test %ax,%ax
39b: 90 nop
39c: 8d 74 26 00 lea 0x0(%esi),%esi
3a0: 74 31 je 3d3 <inet6_create+0xe3>
3a2: 8b 1b mov (%ebx),%ebx
3a4: 89 5c 24 1c mov %ebx,0x1c(%esp)
3a8: 8b 44 24 1c mov 0x1c(%esp),%eax
3ac: 8b 00 mov (%eax),%eax
3ae: 8d 44 20 00 lea 0x0(%eax),%eax
3b2: 0f bf c7 movswl %di,%eax
3b5: 8d 04 c5 00 00 00 00 lea 0x0(,%eax,8),%eax
3b8: R_386_32 .bss
3bc: 39 d8 cmp %ebx,%eax
3be: 0f 84 e9 01 00 00 je 5ad <inet6_create+0x2bd>
3c4: 0f b7 43 0a movzwl 0xa(%ebx),%eax
3c8: 0f b7 c8 movzwl %ax,%ecx
3cb: 39 ce cmp %ecx,%esi
3cd: 75 c1 jne 390 <inet6_create+0xa0>
3cf: 85 f6 test %esi,%esi
3d1: 74 cf je 3a2 <inet6_create+0xb2>
3d3: 8b 43 14 mov 0x14(%ebx),%eax
3d6: 85 c0 test %eax,%eax
3d8: 7e 12 jle 3ec <inet6_create+0xfc>
3da: e8 fc ff ff ff call 3db <inet6_create+0xeb>
3db: R_386_PC32 capable
3df: 85 c0 test %eax,%eax
3e1: bd ff ff ff ff mov $0xffffffff,%ebp
3e6: 0f 84 99 01 00 00 je 585 <inet6_create+0x295>
3ec: 8b 43 10 mov 0x10(%ebx),%eax
3ef: 8b 54 24 10 mov 0x10(%esp),%edx
3f3: b9 ec 03 00 00 mov $0x3ec,%ecx
3f4: R_386_32 .text
3f8: 89 42 08 mov %eax,0x8(%edx)
3fb: 0f b6 43 18 movzbl 0x18(%ebx),%eax
3ff: 8b 7b 0c mov 0xc(%ebx),%edi
402: 88 44 24 17 mov %al,0x17(%esp)
406: 0f b6 53 19 movzbl 0x19(%ebx),%edx
40a: b8 00 00 00 00 mov $0x0,%eax
40b: R_386_32 rcu_lock_map
40f: 88 54 24 16 mov %dl,0x16(%esp)
413: ba 01 00 00 00 mov $0x1,%edx
418: e8 fc ff ff ff call 419 <inet6_create+0x129>
419: R_386_PC32 lock_release
41d: 8b 57 70 mov 0x70(%edi),%edx
420: 85 d2 test %edx,%edx
422: 0f 84 36 02 00 00 je 65e <inet6_create+0x36e>
428: b9 d0 00 00 00 mov $0xd0,%ecx
42d: ba 0a 00 00 00 mov $0xa,%edx
432: b8 00 00 00 00 mov $0x0,%eax
433: R_386_32 init_net
437: 89 3c 24 mov %edi,(%esp)
43a: c7 44 24 04 01 00 00 movl $0x1,0x4(%esp)
441: 00
442: bd 97 ff ff ff mov $0xffffff97,%ebp
447: e8 fc ff ff ff call 448 <inet6_create+0x158>
448: R_386_PC32 sk_alloc
44c: 85 c0 test %eax,%eax
44e: 89 c7 mov %eax,%edi
450: 0f 84 b3 fe ff ff je 309 <inet6_create+0x19>
456: 89 c2 mov %eax,%edx
458: 8b 44 24 10 mov 0x10(%esp),%eax
45c: e8 fc ff ff ff call 45d <inet6_create+0x16d>
45d: R_386_PC32 sock_init_data
461: 80 64 24 17 03 andb $0x3,0x17(%esp)
466: 0f b6 54 24 17 movzbl 0x17(%esp),%edx
46b: 0f b6 47 28 movzbl 0x28(%edi),%eax
46f: c1 e2 02 shl $0x2,%edx
472: 83 e0 f3 and $0xfffffff3,%eax
475: 09 d0 or %edx,%eax
477: 88 47 28 mov %al,0x28(%edi)
47a: 0f b6 44 24 16 movzbl 0x16(%esp),%eax
47f: a8 01 test $0x1,%al
481: 74 04 je 487 <inet6_create+0x197>
483: c6 47 03 01 movb $0x1,0x3(%edi)
487: 0f b6 97 3f 02 00 00 movzbl 0x23f(%edi),%edx
48e: c1 e8 02 shr $0x2,%eax
491: 83 e0 01 and $0x1,%eax
494: 01 c0 add %eax,%eax
496: 83 e2 fd and $0xfffffffd,%edx
499: 09 c2 or %eax,%edx
49b: 88 97 3f 02 00 00 mov %dl,0x23f(%edi)
4a1: 8b 44 24 10 mov 0x10(%esp),%eax
4a5: 66 83 78 3c 03 cmpw $0x3,0x3c(%eax)
4aa: 0f 84 64 01 00 00 je 614 <inet6_create+0x324>
4b0: 89 f2 mov %esi,%edx
4b2: c7 87 18 02 00 00 00 movl $0x0,0x218(%edi)
4b9: 00 00 00
4b8: R_386_32 inet_sock_destruct
4bc: 66 c7 07 0a 00 movw $0xa,(%edi)
4c1: 88 57 29 mov %dl,0x29(%edi)
4c4: 8b 43 0c mov 0xc(%ebx),%eax
4c7: 8b 40 40 mov 0x40(%eax),%eax
4ca: 89 87 14 02 00 00 mov %eax,0x214(%edi)
4d0: 8b 47 20 mov 0x20(%edi),%eax
4d3: 8b 48 74 mov 0x74(%eax),%ecx
4d6: 83 e9 70 sub $0x70,%ecx
4d9: 8d 0c 0f lea (%edi,%ecx,1),%ecx
4dc: 89 8f 1c 02 00 00 mov %ecx,0x21c(%edi)
4e2: 0f b6 41 46 movzbl 0x46(%ecx),%eax
4e6: 66 c7 41 3c ff ff movw $0xffff,0x3c(%ecx)
4ec: 66 c7 41 3e ff ff movw $0xffff,0x3e(%ecx)
4f2: 83 e0 e7 and $0xffffffe7,%eax
4f5: 83 c8 09 or $0x9,%eax
4f8: 88 41 46 mov %al,0x46(%ecx)
4fb: 0f b6 15 00 00 00 00 movzbl 0x0,%edx
4fe: R_386_32 sysctl_ipv6_bindv6only
502: 83 e0 df and $0xffffffdf,%eax
505: 83 e2 01 and $0x1,%edx
508: c1 e2 05 shl $0x5,%edx
50b: 09 d0 or %edx,%eax
50d: 88 41 46 mov %al,0x46(%ecx)
510: 80 8f 3f 02 00 00 10 orb $0x10,0x23f(%edi)
517: 66 c7 87 30 02 00 00 movw $0xffff,0x230(%edi)
51e: ff ff
520: c6 87 3d 02 00 00 01 movb $0x1,0x23d(%edi)
527: c7 87 40 02 00 00 00 movl $0x0,0x240(%edi)
52e: 00 00 00
531: c7 87 48 02 00 00 00 movl $0x0,0x248(%edi)
538: 00 00 00
53b: a1 04 00 00 00 mov 0x4,%eax
53c: R_386_32 ipv4_config
540: 85 c0 test %eax,%eax
542: 0f b7 87 2a 02 00 00 movzwl 0x22a(%edi),%eax
549: 0f 94 87 3e 02 00 00 sete 0x23e(%edi)
550: 66 85 c0 test %ax,%ax
553: 0f 85 a3 00 00 00 jne 5fc <inet6_create+0x30c>
559: 8b 47 20 mov 0x20(%edi),%eax
55c: 31 ed xor %ebp,%ebp
55e: 8b 50 14 mov 0x14(%eax),%edx
561: 85 d2 test %edx,%edx
563: 0f 84 a0 fd ff ff je 309 <inet6_create+0x19>
569: 89 f8 mov %edi,%eax
56b: ff d2 call *%edx
56d: 85 c0 test %eax,%eax
56f: 89 c5 mov %eax,%ebp
571: 0f 84 92 fd ff ff je 309 <inet6_create+0x19>
577: 89 f8 mov %edi,%eax
579: e8 fc ff ff ff call 57a <inet6_create+0x28a>
57a: R_386_PC32 sk_common_release
57e: 66 90 xchg %ax,%ax
580: e9 84 fd ff ff jmp 309 <inet6_create+0x19>
585: b8 00 00 00 00 mov $0x0,%eax
586: R_386_32 rcu_lock_map
58a: b9 85 05 00 00 mov $0x585,%ecx
58b: R_386_32 .text
58f: ba 01 00 00 00 mov $0x1,%edx
594: e8 fc ff ff ff call 595 <inet6_create+0x2a5>
595: R_386_PC32 lock_release
599: 83 c4 20 add $0x20,%esp
59c: 89 e8 mov %ebp,%eax
59e: 5b pop %ebx
59f: 5e pop %esi
5a0: 5f pop %edi
5a1: 5d pop %ebp
5a2: c3 ret
5a3: e8 fc ff ff ff call 5a4 <inet6_create+0x2b4>
5a4: R_386_PC32 build_ehash_secret
5a8: e9 80 fd ff ff jmp 32d <inet6_create+0x3d>
5ad: bd a3 ff ff ff mov $0xffffffa3,%ebp
5b2: 83 7c 24 18 02 cmpl $0x2,0x18(%esp)
5b7: 74 cc je 585 <inet6_create+0x295>
5b9: b9 b9 05 00 00 mov $0x5b9,%ecx
5ba: R_386_32 .text
5be: ba 01 00 00 00 mov $0x1,%edx
5c3: b8 00 00 00 00 mov $0x0,%eax
5c4: R_386_32 rcu_lock_map
5c8: e8 fc ff ff ff call 5c9 <inet6_create+0x2d9>
5c9: R_386_PC32 lock_release
5cd: ff 44 24 18 incl 0x18(%esp)
5d1: 83 7c 24 18 01 cmpl $0x1,0x18(%esp)
5d6: 74 5d je 635 <inet6_create+0x345>
5d8: 89 74 24 08 mov %esi,0x8(%esp)
5dc: c7 44 24 04 0a 00 00 movl $0xa,0x4(%esp)
5e3: 00
5e4: c7 04 24 1b 00 00 00 movl $0x1b,(%esp)
5e7: R_386_32 .rodata.str1.1
5eb: e8 fc ff ff ff call 5ec <inet6_create+0x2fc>
5ec: R_386_PC32 request_module
5f0: e9 40 fd ff ff jmp 335 <inet6_create+0x45>
5f5: 89 ce mov %ecx,%esi
5f7: e9 d7 fd ff ff jmp 3d3 <inet6_create+0xe3>
5fc: 8b 57 20 mov 0x20(%edi),%edx
5ff: 66 c1 c0 08 rol $0x8,%ax
603: 66 89 87 38 02 00 00 mov %ax,0x238(%edi)
60a: 89 f8 mov %edi,%eax
60c: ff 52 44 call *0x44(%edx)
60f: e9 45 ff ff ff jmp 559 <inet6_create+0x269>
614: 81 fe ff 00 00 00 cmp $0xff,%esi
61a: 66 89 b7 2a 02 00 00 mov %si,0x22a(%edi)
621: 0f 85 89 fe ff ff jne 4b0 <inet6_create+0x1c0>
627: 83 ca 08 or $0x8,%edx
62a: 88 97 3f 02 00 00 mov %dl,0x23f(%edi)
630: e9 7b fe ff ff jmp 4b0 <inet6_create+0x1c0>
635: 8b 54 24 10 mov 0x10(%esp),%edx
639: 0f bf 42 3c movswl 0x3c(%edx),%eax
63d: 89 74 24 08 mov %esi,0x8(%esp)
641: c7 44 24 04 0a 00 00 movl $0xa,0x4(%esp)
648: 00
649: c7 04 24 00 00 00 00 movl $0x0,(%esp)
64c: R_386_32 .rodata.str1.1
650: 89 44 24 0c mov %eax,0xc(%esp)
654: e8 fc ff ff ff call 655 <inet6_create+0x365>
655: R_386_PC32 request_module
659: e9 d7 fc ff ff jmp 335 <inet6_create+0x45>
65e: c7 44 24 0c a2 00 00 movl $0xa2,0xc(%esp)
665: 00
666: c7 44 24 08 a0 00 00 movl $0xa0,0x8(%esp)
66d: 00
66a: R_386_32 .rodata.str1.4
66e: c7 44 24 04 2e 00 00 movl $0x2e,0x4(%esp)
675: 00
672: R_386_32 .rodata.str1.1
676: c7 04 24 e0 00 00 00 movl $0xe0,(%esp)
679: R_386_32 .rodata.str1.4
67d: e8 fc ff ff ff call 67e <inet6_create+0x38e>
67e: R_386_PC32 printk
682: e9 a1 fd ff ff jmp 428 <inet6_create+0x138>
687: 89 f6 mov %esi,%esi
689: 8d bc 27 00 00 00 00 lea 0x0(%edi),%edi
00000690 <inet6_destroy_sock>:
^ permalink raw reply
* Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?
From: Dave Johnson @ 2007-11-02 18:08 UTC (permalink / raw)
To: linux-kernel, netdev, bguo, Michael Chan
In-Reply-To: <18218.18134.353553.189622@zeus.sw.starentnetworks.com>
Dave Johnson writes:
> Ben Greear writes:
> > Currently, VLAN devices offer the ability to 'reorder' the header
> > and explicitly remove the VLAN header. I assume we keep this
> > feature and have the AF_PACKET logic check the device flags to see
> > if it should insert the VLAN header for hw-accel vlans?
> >
> > Either way, if we sniff the underlying device, we should always get
> > the VLAN header.
>
> Yes, but it's more than just a packet socket issue.
>
> A quick look through the hwaccel capable drivers (in 2.6.23) and most
> are doing something like:
>
> if (foo->vlgrp && packet_is_tagged)
> vlan_hwaccel_receive_skb(skb, foo->vlgrp, vlan_tag);
> else
> netif_receive_skb(skb);
>
> The important thing here is if the vlan group is NULL, the MAC must
> be configured to NOT strip the tag.
>
> users of NETIF_F_HW_VLAN_RX:
> ---------------------------
> ./drivers/net/8139cp.c: looks ok
> ./drivers/net/acenic.c: *1
> ./drivers/net/amd8111e.c: unsure, probably *1
> ./drivers/net/atl1/atl1_main.c: looks ok
> ./drivers/net/bnx2.c: *2
> ./drivers/net/bonding/bond_main.c: unsure, probably ok
> ./drivers/net/chelsio/cxgb2.c: looks ok
> ./drivers/net/cxgb3/cxgb3_main.c: looks ok
> ./drivers/net/e1000/e1000_main.c: looks ok
> ./drivers/net/ehea/ehea_main.c: unsure, probably ok
> ./drivers/net/forcedeth.c: looks ok
> ./drivers/net/gianfar.c: looks ok
> ./drivers/net/ixgb/ixgb_main.c: looks ok
> ./drivers/net/ns83820.c: unsure, probably ok
> ./drivers/net/r8169.c: looks ok
> ./drivers/net/s2io.c: *1
> ./drivers/net/sky2.c: looks ok
> ./drivers/net/starfire.c: unsure, probably ok
> ./drivers/net/tg3.c: *2
> ./drivers/net/typhoon.c: unsure, probably ok
> ./drivers/s390/net/qeth_main.c: unsure, probably ok
>
> *1: Driver configures the MAC to strip TAGs even if vlan group is
> NULL. MAC strips the tag, but driver calls netif_rx() or
> netif_receive_skb() with the packet as untagged. Kernel
> processes tagged packet as if it was received untagged. Possible
> security issue.
>
> *2: If chip supports 'ASF', tag is always stripped (see *1 above).
> Looks ok if ASF is not supported.
Michael,
These changes seems to cause this issue:
> [BNX2]: Fix VLAN on ASF
>
> Always set up the device to strip incoming VLAN tags when ASF is
> enabled. ASF firmware will not parse packets correctly if VLAN tags
> are not stripped.
>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> GIT: e29054f92d7d575631691865c1b95bee5bc974cc
and
> ChangeSet@1.1371.72.2, 2003-12-02 02:34:13-08:00, davem@nuts.ninka.net +1 -0
> [TG3]: Do not set RX_MODE_KEEP_VLAN_TAG when ASF is enabled.
Could you elaborate if this is really needed, if so is there some
workaround that could be done instead?
Simply removing the check seemed to work for me, but I'm unsure if
this is actually a valid thing to do with these MACs.
--
Dave Johnson
Starent Networks
^ permalink raw reply
* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: David Brownell @ 2007-11-02 18:45 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <20071101234443.GD7227@stusta.de>
On Thursday 01 November 2007, Adrian Bunk wrote:
> The following combination of options is simply an unusual one:
>
> CONFIG_MII=m
> CONFIG_USB_USBNET=y
> CONFIG_USB_USBNET_MII=n
I though that had been fixed for ages ...
This should do a better job of it.
- Dave
========== CUT HERE
Simplify handling of the MII-dependent usbnet based adapters: stick
to forward dependencies, and explicitly handle the core dependency.
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
drivers/net/usb/Kconfig | 23 ++++++++++++-----------
drivers/net/usb/usbnet.c | 9 ++++++++-
2 files changed, 20 insertions(+), 12 deletions(-)
--- a.orig/drivers/net/usb/Kconfig 2007-10-21 10:35:16.000000000 -0700
+++ a/drivers/net/usb/Kconfig 2007-11-02 11:32:15.000000000 -0700
@@ -93,13 +93,8 @@ config USB_RTL8150
To compile this driver as a module, choose M here: the
module will be called rtl8150.
-config USB_USBNET_MII
- tristate
- default n
-
config USB_USBNET
tristate "Multi-purpose USB Networking Framework"
- select MII if USB_USBNET_MII != n
---help---
This driver supports several kinds of network links over USB,
with "minidrivers" built around a common network driver core
@@ -131,11 +126,19 @@ config USB_USBNET
To compile this driver as a module, choose M here: the
module will be called usbnet.
+# usbnet core will support MII when MII is static, or both are modules
+config USB_NET_MII
+ tristate
+ depends on USB_USBNET && NET_ETHERNET && (MII = y || MII = USB_USBNET)
+ default MII
+
+comment "MII support is needed for most Ethernet adapters"
+ depends on USB_USBNET && USB_NET_MII=n
+
config USB_NET_AX8817X
tristate "ASIX AX88xxx Based USB 2.0 Ethernet Adapters"
- depends on USB_USBNET && NET_ETHERNET
+ depends on USB_USBNET && USB_NET_MII
select CRC32
- select USB_USBNET_MII
default y
help
This option adds support for ASIX AX88xxx based USB 2.0
@@ -188,9 +191,8 @@ config USB_NET_CDCETHER
config USB_NET_DM9601
tristate "Davicom DM9601 based USB 1.1 10/100 ethernet devices"
- depends on USB_USBNET
+ depends on USB_USBNET && USB_NET_MII
select CRC32
- select USB_USBNET_MII
help
This option adds support for Davicom DM9601 based USB 1.1
10/100 Ethernet adapters.
@@ -224,8 +226,7 @@ config USB_NET_PLUSB
config USB_NET_MCS7830
tristate "MosChip MCS7830 based Ethernet adapters"
- depends on USB_USBNET
- select USB_USBNET_MII
+ depends on USB_USBNET && USB_NET_MII
help
Choose this option if you're using a 10/100 Ethernet USB2
adapter based on the MosChip 7830 controller. This includes
--- a.orig/drivers/net/usb/usbnet.c 2007-10-13 15:16:10.000000000 -0700
+++ a/drivers/net/usb/usbnet.c 2007-11-02 11:39:59.000000000 -0700
@@ -682,10 +682,17 @@ done_nopm:
/* ethtool methods; minidrivers may need to add some more, but
* they'll probably want to use this base set.
*/
+#undef HAVE_MII
-#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
+#if defined(CONFIG_MII)
#define HAVE_MII
+#elif defined(CONFIG_MII_MODULE) && defined(MODULE)
+#define HAVE_MII
+#endif
+
+#ifdef HAVE_MII
+
int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
{
struct usbnet *dev = netdev_priv(net);
^ permalink raw reply
* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: Adrian Bunk @ 2007-11-02 18:57 UTC (permalink / raw)
To: David Brownell; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <200711021145.15542.david-b@pacbell.net>
On Fri, Nov 02, 2007 at 11:45:15AM -0700, David Brownell wrote:
> On Thursday 01 November 2007, Adrian Bunk wrote:
> > The following combination of options is simply an unusual one:
> >
> > CONFIG_MII=m
> > CONFIG_USB_USBNET=y
> > CONFIG_USB_USBNET_MII=n
>
> I though that had been fixed for ages ...
>
> This should do a better job of it.
>
> - Dave
>
> ========== CUT HERE
> Simplify handling of the MII-dependent usbnet based adapters: stick
> to forward dependencies, and explicitly handle the core dependency.
>
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---
> drivers/net/usb/Kconfig | 23 ++++++++++++-----------
> drivers/net/usb/usbnet.c | 9 ++++++++-
> 2 files changed, 20 insertions(+), 12 deletions(-)
>
> --- a.orig/drivers/net/usb/Kconfig 2007-10-21 10:35:16.000000000 -0700
> +++ a/drivers/net/usb/Kconfig 2007-11-02 11:32:15.000000000 -0700
> @@ -93,13 +93,8 @@ config USB_RTL8150
> To compile this driver as a module, choose M here: the
> module will be called rtl8150.
>
> -config USB_USBNET_MII
> - tristate
> - default n
> -
> config USB_USBNET
> tristate "Multi-purpose USB Networking Framework"
> - select MII if USB_USBNET_MII != n
> ---help---
> This driver supports several kinds of network links over USB,
> with "minidrivers" built around a common network driver core
> @@ -131,11 +126,19 @@ config USB_USBNET
> To compile this driver as a module, choose M here: the
> module will be called usbnet.
>
> +# usbnet core will support MII when MII is static, or both are modules
> +config USB_NET_MII
> + tristate
> + depends on USB_USBNET && NET_ETHERNET && (MII = y || MII = USB_USBNET)
> + default MII
> +
> +comment "MII support is needed for most Ethernet adapters"
> + depends on USB_USBNET && USB_NET_MII=n
>...
This approach has two disadvantages:
- it's complicated
- the MII stuff is an implementation detail, and we shouldn't bother
the user with it (especially since we can do better)
If you want to keep the #ifdef's, what's the problem with the second
patch I proposed to fix this bug?
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* [PATCH] NET: Remove unneeded type cast in skb_truesize_check()
From: Chuck Lever @ 2007-11-02 19:14 UTC (permalink / raw)
To: netdev
The (int) type cast in skb_truesize_check() is unneeded: without it, all
the variable types in the conditional expression are unsigned integers. As
it stands, the type cast causes a comparison between a signed and an
unsigned integer, which can produce unexpected results.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
include/linux/skbuff.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 94e4991..7965216 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -387,7 +387,7 @@ extern void skb_truesize_bug(struct sk_buff *skb);
static inline void skb_truesize_check(struct sk_buff *skb)
{
- if (unlikely((int)skb->truesize < sizeof(struct sk_buff) + skb->len))
+ if (unlikely(skb->truesize < sizeof(struct sk_buff) + skb->len))
skb_truesize_bug(skb);
}
^ permalink raw reply related
* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: David Brownell @ 2007-11-02 19:30 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <20071102185751.GV30287@stusta.de>
On Friday 02 November 2007, Adrian Bunk wrote:
> This approach has two disadvantages:
> - it's complicated
No more so than the problem itself.
> - the MII stuff is an implementation detail, and we shouldn't bother
> the user with it (especially since we can do better)
That's a Kconfig policy that's not always followed. In this
case, I was getting fed up with "select". It so rarely does
what it needs to do, and I've started to think it'd be better
to just always avoid that fragility than battle it.
> If you want to keep the #ifdef's, what's the problem with the second
> patch I proposed to fix this bug?
For one thing, I didn't see it until after I posted this one...
other than that, the basic approach could well be fine; I didn't
go through it in detail.
But on the other hand, it seems that only the ASIX code will work
right; the DM9601 and MCS7830 Kconfig is different/wrong.
- Dave
^ permalink raw reply
* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: Adrian Bunk @ 2007-11-02 19:55 UTC (permalink / raw)
To: David Brownell; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <200711021230.22588.david-b@pacbell.net>
On Fri, Nov 02, 2007 at 12:30:22PM -0700, David Brownell wrote:
> On Friday 02 November 2007, Adrian Bunk wrote:
> > This approach has two disadvantages:
> > - it's complicated
>
> No more so than the problem itself.
>
>
> > - the MII stuff is an implementation detail, and we shouldn't bother
> > the user with it (especially since we can do better)
>
> That's a Kconfig policy that's not always followed.
Sure it's not yet always followed.
But kernel developers have to become more aware that the vast majority
of kconfig users are not kernel hackers and act accordingly.
I'm not talking about the infamous Aunt Tillie, but being able to
build your own kernel is even required for LPIC-1. [1]
> In this
> case, I was getting fed up with "select". It so rarely does
> what it needs to do, and I've started to think it'd be better
> to just always avoid that fragility than battle it.
Regarding this bug, "select" is completely innocent...
> > If you want to keep the #ifdef's, what's the problem with the second
> > patch I proposed to fix this bug?
>
> For one thing, I didn't see it until after I posted this one...
> other than that, the basic approach could well be fine; I didn't
> go through it in detail.
>
> But on the other hand, it seems that only the ASIX code will work
> right; the DM9601 and MCS7830 Kconfig is different/wrong.
I'm not seeing the problem.
Which configuration will be handled wrongly?
> - Dave
cu
Adrian
[1] http://www.lpi.org/en/lpi/english/certification/the_lpic_program/exam_102_detailed_objectives
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* Re: build #337 failed for 2.6.24-rc1-gb1d08ac In function `usbnet_set_settings':
From: Adrian Bunk @ 2007-11-02 20:05 UTC (permalink / raw)
To: David Brownell; +Cc: Randy Dunlap, Toralf Förster, linux-kernel, netdev
In-Reply-To: <200711021145.15542.david-b@pacbell.net>
On Fri, Nov 02, 2007 at 11:45:15AM -0700, David Brownell wrote:
>...
> --- a.orig/drivers/net/usb/usbnet.c 2007-10-13 15:16:10.000000000 -0700
> +++ a/drivers/net/usb/usbnet.c 2007-11-02 11:39:59.000000000 -0700
> @@ -682,10 +682,17 @@ done_nopm:
> /* ethtool methods; minidrivers may need to add some more, but
> * they'll probably want to use this base set.
> */
> +#undef HAVE_MII
>
> -#if defined(CONFIG_MII) || defined(CONFIG_MII_MODULE)
> +#if defined(CONFIG_MII)
> #define HAVE_MII
>
> +#elif defined(CONFIG_MII_MODULE) && defined(MODULE)
> +#define HAVE_MII
> +#endif
> +
> +#ifdef HAVE_MII
> +
> int usbnet_get_settings (struct net_device *net, struct ethtool_cmd *cmd)
> {
> struct usbnet *dev = netdev_priv(net);
>
Despite all what you do in Kconfig and what you wrongly blame on
"select" the bug is in usbnet.c and this fix to usbnet.c _alone_
would be enough to fix the bug.
But since you said you care about not including bloat you should better
take my second patch that results in smaller code in some configurations.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* [PATCH] NET: Make ts_recent_stamp and tw_ts_recent_stamp unsigned long
From: Chuck Lever @ 2007-11-02 20:21 UTC (permalink / raw)
To: netdev
The get_seconds() function now returns an unsigned long. K&R states
that comparing a signed long with an unsigned long implicitly converts
the signed long to unsigned long [1].
Thus ts_recent_stamp and tw_ts_recent_stamp are already implicitly
converted to unsigned long in every comparison with get_seconds() or
other unsigned longs (such as tcp_ts_stamp).
Code review shows that ts_recent_stamp is compared only to get_seconds() or
zero. It is set only to zero, the return value of get_seconds(), or the
value of tw_ts_recent_stamp. Thus changing ts_recent_stamp to an unsigned
long does not change current behavior in any way, and eliminates implicit
type conversions in several places, including some comparisons.
Review shows that tw_ts_recent_stamp is set only to the return value of
get_seconds() or to the value of ts_recent_stamp. It is compared to the
return value of get_seconds(), the value of tcp_ts_stamp (an unsigned
long), one, or zero. Thus changing tw_ts_recent_stamp to an unsigned long
also does not change current behavior in any way.
It might improve matters even further if these open-coded timestamp
comparisons were replaced with something similar to the time_after_eq
macro from include/linux/jiffies.h.
[1] Kernighan & Ritchie, "The C Programming Language", 2nd. ed., appendix
A, sec. 6.5.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Cc: netdev@vger.kernel.org
---
include/linux/tcp.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index bac17c5..0122374 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -206,7 +206,7 @@ struct tcp_sack_block {
struct tcp_options_received {
/* PAWS/RTTM data */
- long ts_recent_stamp;/* Time we stored ts_recent (for aging) */
+ unsigned long ts_recent_stamp;/* Time we stored ts_recent (for aging) */
u32 ts_recent; /* Time stamp to echo next */
u32 rcv_tsval; /* Time stamp value */
u32 rcv_tsecr; /* Time stamp echo reply */
@@ -419,7 +419,7 @@ struct tcp_timewait_sock {
u32 tw_snd_nxt;
u32 tw_rcv_wnd;
u32 tw_ts_recent;
- long tw_ts_recent_stamp;
+ unsigned long tw_ts_recent_stamp;
#ifdef CONFIG_TCP_MD5SIG
u16 tw_md5_keylen;
u8 tw_md5_key[TCP_MD5SIG_MAXKEYLEN];
^ permalink raw reply related
* Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?
From: Michael Chan @ 2007-11-02 21:52 UTC (permalink / raw)
To: Dave Johnson; +Cc: linux-kernel, netdev, bguo
In-Reply-To: <18219.26528.485099.317944@zeus.sw.starentnetworks.com>
On Fri, 2007-11-02 at 14:08 -0400, Dave Johnson wrote:
> >
> > *2: If chip supports 'ASF', tag is always stripped (see *1 above).
> > Looks ok if ASF is not supported.
>
> Michael,
>
> These changes seems to cause this issue:
>
> > [BNX2]: Fix VLAN on ASF
> >
> > Always set up the device to strip incoming VLAN tags when ASF is
> > enabled. ASF firmware will not parse packets correctly if VLAN tags
> > are not stripped.
> >
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > GIT: e29054f92d7d575631691865c1b95bee5bc974cc
>
> and
>
> > ChangeSet@1.1371.72.2, 2003-12-02 02:34:13-08:00, davem@nuts.ninka.net +1 -0
> > [TG3]: Do not set RX_MODE_KEEP_VLAN_TAG when ASF is enabled.
>
>
> Could you elaborate if this is really needed, if so is there some
> workaround that could be done instead?
This is needed for management firmware to work properly. Management
firmware expects any VLAN tags to be stripped. Unfortunately, VLAN
stripping cannot be done independently between the driver and the
firmware. The workaround is to disable management firmware.
>
> Simply removing the check seemed to work for me, but I'm unsure if
> this is actually a valid thing to do with these MACs.
Most of these on-board devices are shipped with management firmware
enabled. Removing the check will make the firmware not functional.
We realize this VLAN limitation is causing problems to many users and we
are looking for ways to address it.
^ permalink raw reply
* Re: expected behavior of PF_PACKET on NETIF_F_HW_VLAN_RX device?
From: David Miller @ 2007-11-02 21:20 UTC (permalink / raw)
To: djohnson+linux-kernel; +Cc: linux-kernel, netdev, bguo, mchan
In-Reply-To: <18219.26528.485099.317944@zeus.sw.starentnetworks.com>
From: Dave Johnson <djohnson+linux-kernel@sw.starentnetworks.com>
Date: Fri, 2 Nov 2007 14:08:32 -0400
> These changes seems to cause this issue:
>
> > [BNX2]: Fix VLAN on ASF
> >
> > Always set up the device to strip incoming VLAN tags when ASF is
> > enabled. ASF firmware will not parse packets correctly if VLAN tags
> > are not stripped.
> >
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > GIT: e29054f92d7d575631691865c1b95bee5bc974cc
>
> and
>
> > ChangeSet@1.1371.72.2, 2003-12-02 02:34:13-08:00, davem@nuts.ninka.net +1 -0
> > [TG3]: Do not set RX_MODE_KEEP_VLAN_TAG when ASF is enabled.
>
>
> Could you elaborate if this is really needed, if so is there some
> workaround that could be done instead?
>
> Simply removing the check seemed to work for me, but I'm unsure if
> this is actually a valid thing to do with these MACs.
Unfortunately the ASF firmware is very picky.
I think were are stuck with this behavior.
^ permalink raw reply
* Re: [PATCH] NET: Remove unneeded type cast in skb_truesize_check()
From: David Miller @ 2007-11-02 21:27 UTC (permalink / raw)
To: chuck.lever; +Cc: netdev
In-Reply-To: <20071102191426.30464.7490.stgit@manray.1015granger.net>
From: Chuck Lever <chuck.lever@oracle.com>
Date: Fri, 02 Nov 2007 15:14:26 -0400
> The (int) type cast in skb_truesize_check() is unneeded: without it, all
> the variable types in the conditional expression are unsigned integers. As
> it stands, the type cast causes a comparison between a signed and an
> unsigned integer, which can produce unexpected results.
>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
This is checking for skb->truesize being decremented too much by other
code, which could cause a wraparound below zero, so we do want
negaitve checks here.
Please stop these mindless signedness conversions without first trying
to think about what the code might be trying to do.
Thank you.
^ permalink raw reply
* Re: bizarre network timing problem
From: Felix von Leitner @ 2007-11-02 22:11 UTC (permalink / raw)
To: Rick Jones; +Cc: Chuck Ebbert, linux-kernel, Netdev
In-Reply-To: <4717964A.8080100@hp.com>
Thus spake Rick Jones (rick.jones2@hp.com):
> >How could I test this theory?
> Can you take another trace that isn't so "cooked?" One that just sticks
> with TCP-level and below stuff?
Sorry for taking so long. Here is a tcpdump. The side on port 445 is
the SMB server using TCP_CORK.
23:03:20.283772 IP 127.0.0.1.33230 > 127.0.0.1.445: S 1503927325:1503927325(0) win 32792 <mss 16396,sackOK,timestamp 9451736 0,nop,wscale 7>
23:03:20.283774 IP 127.0.0.1.445 > 127.0.0.1.33230: S 1513925692:1513925692(0) ack 1503927326 win 32768 <mss 16396,sackOK,timestamp 9451737 9451736,nop,wscale 7>
23:03:20.283797 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 1 win 257 <nop,nop,timestamp 9451737 9451737>
23:03:20.295851 IP 127.0.0.1.33230 > 127.0.0.1.445: P 1:195(194) ack 1 win 257 <nop,nop,timestamp 9451740 9451737>
23:03:20.295881 IP 127.0.0.1.445 > 127.0.0.1.33230: . ack 195 win 265 <nop,nop,timestamp 9451740 9451740>
23:03:20.295959 IP 127.0.0.1.445 > 127.0.0.1.33230: P 1:87(86) ack 195 win 265 <nop,nop,timestamp 9451740 9451740>
23:03:20.295998 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 87 win 256 <nop,nop,timestamp 9451740 9451740>
23:03:20.296063 IP 127.0.0.1.33230 > 127.0.0.1.445: P 195:287(92) ack 87 win 256 <nop,nop,timestamp 9451740 9451740>
23:03:20.296096 IP 127.0.0.1.445 > 127.0.0.1.33230: P 87:181(94) ack 287 win 265 <nop,nop,timestamp 9451740 9451740>
23:03:20.296135 IP 127.0.0.1.33230 > 127.0.0.1.445: P 287:373(86) ack 181 win 255 <nop,nop,timestamp 9451740 9451740>
23:03:20.296163 IP 127.0.0.1.445 > 127.0.0.1.33230: P 181:239(58) ack 373 win 265 <nop,nop,timestamp 9451740 9451740>
23:03:20.296201 IP 127.0.0.1.33230 > 127.0.0.1.445: P 373:459(86) ack 239 win 255 <nop,nop,timestamp 9451740 9451740>
23:03:20.296245 IP 127.0.0.1.445 > 127.0.0.1.33230: P 239:309(70) ack 459 win 265 <nop,nop,timestamp 9451740 9451740>
23:03:20.296286 IP 127.0.0.1.33230 > 127.0.0.1.445: P 459:535(76) ack 309 win 254 <nop,nop,timestamp 9451740 9451740>
23:03:20.296314 IP 127.0.0.1.445 > 127.0.0.1.33230: P 309:461(152) ack 535 win 265 <nop,nop,timestamp 9451740 9451740>
23:03:20.296361 IP 127.0.0.1.33230 > 127.0.0.1.445: P 535:594(59) ack 461 win 253 <nop,nop,timestamp 9451740 9451740>
23:03:20.296400 IP 127.0.0.1.445 > 127.0.0.1.33230: . 461:16845(16384) ack 594 win 265 <nop,nop,timestamp 9451740 9451740>
23:03:20.335748 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 16845 win 125 <nop,nop,timestamp 9451750 9451740>
[note the .2 sec pause]
23:03:20.547763 IP 127.0.0.1.445 > 127.0.0.1.33230: P 16845:32845(16000) ack 594 win 265 <nop,nop,timestamp 9451803 9451750>
23:03:20.547797 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 32845 win 0 <nop,nop,timestamp 9451803 9451803>
23:03:20.547855 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 32845 win 96 <nop,nop,timestamp 9451803 9451803>
23:03:20.547863 IP 127.0.0.1.445 > 127.0.0.1.33230: P 32845:33229(384) ack 594 win 265 <nop,nop,timestamp 9451803 9451803>
23:03:20.547890 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 33229 win 96 <nop,nop,timestamp 9451803 9451803>
[note the .2 sec pause]
23:03:20.755775 IP 127.0.0.1.445 > 127.0.0.1.33230: P 33229:45517(12288) ack 594 win 265 <nop,nop,timestamp 9451855 9451803>
23:03:20.755855 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 45517 win 96 <nop,nop,timestamp 9451855 9451855>
23:03:20.755868 IP 127.0.0.1.445 > 127.0.0.1.33230: P 45517:49613(4096) ack 594 win 265 <nop,nop,timestamp 9451855 9451855>
23:03:20.755898 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 49613 win 96 <nop,nop,timestamp 9451855 9451855>
[another one]
23:03:20.963789 IP 127.0.0.1.445 > 127.0.0.1.33230: P 49613:61901(12288) ack 594 win 265 <nop,nop,timestamp 9451907 9451855>
23:03:20.963871 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 61901 win 96 <nop,nop,timestamp 9451907 9451907>
23:03:20.963885 IP 127.0.0.1.445 > 127.0.0.1.33230: P 61901:64525(2624) ack 594 win 265 <nop,nop,timestamp 9451907 9451907>
23:03:20.963909 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 64525 win 96 <nop,nop,timestamp 9451907 9451907>
23:03:20.964101 IP 127.0.0.1.33230 > 127.0.0.1.445: P 594:653(59) ack 64525 win 96 <nop,nop,timestamp 9451907 9451907>
23:03:21.003790 IP 127.0.0.1.445 > 127.0.0.1.33230: . ack 653 win 265 <nop,nop,timestamp 9451917 9451907>
23:03:21.171811 IP 127.0.0.1.445 > 127.0.0.1.33230: P 64525:76813(12288) ack 653 win 265 <nop,nop,timestamp 9451959 9451907>
You get the idea.
Anyway, now THIS is the interesting case, because we have two packets in
the answer, and you see the first half of the answer leaving immediately
(when I wanted the whole answer to be sent) but the second only leaving
after the .2 sec delay.
> If SMB is a one-request-at-a-time protocol (I can never remember),
It is.
> you
> could simulate it with a netperf TCP_RR test by passing suitable values to
> the test-specific -r option:
> netperf -H <remote> -t TCP_RR -- -r <req>,<rsp>
> If that shows similar behaviour then you can ass-u-me it isn't your
> application.
Oh I'm pretty sure it's not my application, because my application performs
well over ethernet, which is after all its purpose. Also I see the
write, the TCP uncork, then a pause, and then the packet leaving.
Felix
^ permalink raw reply
* Re: TCP_DEFER_ACCEPT issues
From: Felix von Leitner @ 2007-11-02 22:19 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, Linux Netdev List
In-Reply-To: <472AD0AE.50106@cosmosbay.com>
Thus spake Eric Dumazet (dada1@cosmosbay.com):
> 1) Setting a timeout in a millisecond range (< 1000) is not very good
> because some clients may need much more time to send your server the data
> (very long distance). So a second granularity is OK.
I want millisecond accuracy for consistency. select and poll have it,
we have a 1000 Hz timer, we should also expose that accuracy. I don't
want to have sub second timeouts, in case you were wondering.
> 2) After timeout is elapsed, the server tcp stack has no socket associated
> to your client attempt. So closing the server listening socket wont be able
> to send RST. I agree a RST *should* be sent by the server once the timeout
> is triggered.
I don't see any evidence for a timeout happening at all.
I passed 1 as argument to the setsockopt, so I'd expect a timeout to
happen pretty quickly. There was no connection reset until I Ctrl-C'd
the server 15 minuets (!) laster.
> A typical tcpdump of what is happening for a tcp_defer_accept timeout of 20
> seconds is :
> [1]08:52:47.480291 IP client.60930 > server.http: S
> 2498995442:2498995442(0) win 5840 <mss 1460,sackOK,timestamp 2685904595
> 0,nop,wscale 2>
> [2]08:52:47.480302 IP server.http > client.60930: S
> 1173302644:1173302644(0) ack 2498995443 win 5840 <mss 1460>
> [3]08:52:47.481669 IP client.60930 > server.http: . ack 1 win 5840
> [4]08:52:50.757543 IP server.http > client.60930: S
> 1173302644:1173302644(0) ack 2498995443 win 5840 <mss 1460>
> [5]08:52:50.758953 IP client.60930 > server.http: . ack 1 win 5840
> [6]08:52:56.760611 IP server.http > client.60930: S
> 1173302644:1173302644(0) ack 2498995443 win 5840 <mss 1460>
> [7]08:52:56.761886 IP client.60930 > server.http: . ack 1 win 5840
> [8]08:53:08.771254 IP server.http > client.60930: S
> 1173302644:1173302644(0) ack 2498995443 win 5840 <mss 1460>
> [9]08:53:08.772514 IP client.60930 > server.http: . ack 1 win 5840
> [10]08:53:32.782488 IP server.http > client.60930: S
> 1173302644:1173302644(0) ack 2498995443 win 5840 <mss 1460>
> [11]08:53:32.783754 IP client.60930 > server.http: . ack 1 win 5840
> <a very long time, then client finally sends 2 bytes>
> [12]08:59:30.509097 IP client.60930 > server.http: P 1:3(2) ack 1 win 5840
> [13]08:59:30.509125 IP server.http > client.60930: R
> 1173302645:1173302645(0) win 0
I see this, too. If I connect and not send something, I expected the
kernel to drop the connection when the timeout is reached. Nothing like
that happens.
> So TCP_DEFER_ACCEPT might send way more packets than needed.
Only in the face of attackers, and after the handshake. I could live
with that. If the timeout happened.
> We only should wait for the data coming from the client to be able to pass
> the new socket to the listening application.
Yes. And we should send a RST if no data is coming in within the
timeout, which is not happening for me (2.6.23).
Felix
^ permalink raw reply
* Re: bizarre network timing problem
From: Rick Jones @ 2007-11-02 22:33 UTC (permalink / raw)
To: Felix von Leitner; +Cc: Chuck Ebbert, linux-kernel, Netdev
In-Reply-To: <20071102221146.GA4354@codeblau.de>
Felix von Leitner wrote:
> Thus spake Rick Jones (rick.jones2@hp.com):
>
>>>How could I test this theory?
>>
>>Can you take another trace that isn't so "cooked?" One that just sticks
>>with TCP-level and below stuff?
>
>
> Sorry for taking so long. Here is a tcpdump. The side on port 445 is
> the SMB server using TCP_CORK.
>
> 23:03:20.283772 IP 127.0.0.1.33230 > 127.0.0.1.445: S 1503927325:1503927325(0) win 32792 <mss 16396,sackOK,timestamp 9451736 0,nop,wscale 7>
> 23:03:20.283774 IP 127.0.0.1.445 > 127.0.0.1.33230: S 1513925692:1513925692(0) ack 1503927326 win 32768 <mss 16396,sackOK,timestamp 9451737 9451736,nop,wscale 7>
>
> 23:03:20.283797 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 1 win 257 <nop,nop,timestamp 9451737 9451737>
> 23:03:20.295851 IP 127.0.0.1.33230 > 127.0.0.1.445: P 1:195(194) ack 1 win 257 <nop,nop,timestamp 9451740 9451737>
> 23:03:20.295881 IP 127.0.0.1.445 > 127.0.0.1.33230: . ack 195 win 265 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.295959 IP 127.0.0.1.445 > 127.0.0.1.33230: P 1:87(86) ack 195 win 265 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.295998 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 87 win 256 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296063 IP 127.0.0.1.33230 > 127.0.0.1.445: P 195:287(92) ack 87 win 256 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296096 IP 127.0.0.1.445 > 127.0.0.1.33230: P 87:181(94) ack 287 win 265 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296135 IP 127.0.0.1.33230 > 127.0.0.1.445: P 287:373(86) ack 181 win 255 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296163 IP 127.0.0.1.445 > 127.0.0.1.33230: P 181:239(58) ack 373 win 265 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296201 IP 127.0.0.1.33230 > 127.0.0.1.445: P 373:459(86) ack 239 win 255 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296245 IP 127.0.0.1.445 > 127.0.0.1.33230: P 239:309(70) ack 459 win 265 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296286 IP 127.0.0.1.33230 > 127.0.0.1.445: P 459:535(76) ack 309 win 254 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296314 IP 127.0.0.1.445 > 127.0.0.1.33230: P 309:461(152) ack 535 win 265 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296361 IP 127.0.0.1.33230 > 127.0.0.1.445: P 535:594(59) ack 461 win 253 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.296400 IP 127.0.0.1.445 > 127.0.0.1.33230: . 461:16845(16384) ack 594 win 265 <nop,nop,timestamp 9451740 9451740>
> 23:03:20.335748 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 16845 win 125 <nop,nop,timestamp 9451750 9451740>
>
> [note the .2 sec pause]
I wonder if the ack 16384 win 125 not updating the window is part of it? With a
window scale of 7, the advertised window of 125 is only 16000 bytes, and it
looks based on what follows that TCP has another 16384 to send, so my guess is
that TCP was waiting to have enough window, the persist timer expired and TCP
then had to say "oh well, send what I can" Probably a coupling with this being
less than the MSS (16396) involved too.
> 23:03:20.547763 IP 127.0.0.1.445 > 127.0.0.1.33230: P 16845:32845(16000) ack 594 win 265 <nop,nop,timestamp 9451803 9451750>
> 23:03:20.547797 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 32845 win 0 <nop,nop,timestamp 9451803 9451803>
Notice that an ACK comes-back with a zero window in it - that means that by this
point the receiver still hasn't consumed the 16384+16000 bytes sent to id.
> 23:03:20.547855 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 32845 win 96 <nop,nop,timestamp 9451803 9451803>
Now the receiver has pulled some data, on the order of 96*128 bytes so TCP can
now go ahead and send the remaining 384 bytes.
> 23:03:20.547863 IP 127.0.0.1.445 > 127.0.0.1.33230: P 32845:33229(384) ack 594 win 265 <nop,nop,timestamp 9451803 9451803>
> 23:03:20.547890 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 33229 win 96 <nop,nop,timestamp 9451803 9451803>
>
> [note the .2 sec pause]
I'll bet that 96 * 128 is 12288 and we have another persist timer expiring.
I also wonder if the behaviour might be different if you were using send()
rather than sendfile() - just random musings...
> 23:03:20.755775 IP 127.0.0.1.445 > 127.0.0.1.33230: P 33229:45517(12288) ack 594 win 265 <nop,nop,timestamp 9451855 9451803>
> 23:03:20.755855 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 45517 win 96 <nop,nop,timestamp 9451855 9451855>
> 23:03:20.755868 IP 127.0.0.1.445 > 127.0.0.1.33230: P 45517:49613(4096) ack 594 win 265 <nop,nop,timestamp 9451855 9451855>
> 23:03:20.755898 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 49613 win 96 <nop,nop,timestamp 9451855 9451855>
>
> [another one]
>
> 23:03:20.963789 IP 127.0.0.1.445 > 127.0.0.1.33230: P 49613:61901(12288) ack 594 win 265 <nop,nop,timestamp 9451907 9451855>
> 23:03:20.963871 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 61901 win 96 <nop,nop,timestamp 9451907 9451907>
> 23:03:20.963885 IP 127.0.0.1.445 > 127.0.0.1.33230: P 61901:64525(2624) ack 594 win 265 <nop,nop,timestamp 9451907 9451907>
> 23:03:20.963909 IP 127.0.0.1.33230 > 127.0.0.1.445: . ack 64525 win 96 <nop,nop,timestamp 9451907 9451907>
> 23:03:20.964101 IP 127.0.0.1.33230 > 127.0.0.1.445: P 594:653(59) ack 64525 win 96 <nop,nop,timestamp 9451907 9451907>
> 23:03:21.003790 IP 127.0.0.1.445 > 127.0.0.1.33230: . ack 653 win 265 <nop,nop,timestamp 9451917 9451907>
> 23:03:21.171811 IP 127.0.0.1.445 > 127.0.0.1.33230: P 64525:76813(12288) ack 653 win 265 <nop,nop,timestamp 9451959 9451907>
>
> You get the idea.
>
> Anyway, now THIS is the interesting case, because we have two packets in
> the answer, and you see the first half of the answer leaving immediately
> (when I wanted the whole answer to be sent) but the second only leaving
> after the .2 sec delay.
And it wasn't waiting for an ACK/window-update. You could try:
ifconfig lo mtu 1500
and see what happens then.
>>If SMB is a one-request-at-a-time protocol (I can never remember),
> It is.
Joy.
>> you could simulate it with a netperf TCP_RR test by passing suitable values
>> to the test-specific -r option:
>
>>netperf -H <remote> -t TCP_RR -- -r <req>,<rsp>
>
>>If that shows similar behaviour then you can ass-u-me it isn't your
>>application.
>
>
> Oh I'm pretty sure it's not my application, because my application performs
> well over ethernet, which is after all its purpose. Also I see the
> write, the TCP uncork, then a pause, and then the packet leaving.
Well, a wise old engineer tried to teach me that the proper spelling is ass-u-me
:) so just for grins, you might try the TCP_RR test anyway :) And even if your
application is correct (although I wonder why the receiver isn't sucking
data-out very quickly...) if you can reproduce the problem with netperf it will
be easier for others to do so.
rick jones
^ permalink raw reply
* Re: bizarre network timing problem
From: Felix von Leitner @ 2007-11-02 22:38 UTC (permalink / raw)
To: Rick Jones; +Cc: Chuck Ebbert, linux-kernel, Netdev
In-Reply-To: <472BA5BC.6000901@hp.com>
Thus spake Rick Jones (rick.jones2@hp.com):
> >Oh I'm pretty sure it's not my application, because my application performs
> >well over ethernet, which is after all its purpose. Also I see the
> >write, the TCP uncork, then a pause, and then the packet leaving.
> Well, a wise old engineer tried to teach me that the proper spelling is
> ass-u-me :) so just for grins, you might try the TCP_RR test anyway :) And
> even if your application is correct (although I wonder why the receiver
> isn't sucking data-out very quickly...) if you can reproduce the problem
> with netperf it will be easier for others to do so.
My application is only the server, the receiver is smbget from Samba, so
I don't feel responsible for it :-)
Still, when run over Ethernet, it works fine without waiting for
timeouts to expire.
To reproduce this:
- smbget is from samba, you probably already have this
- gatling (my server) can be gotten from
cvs -d :pserver:cvs@cvs.fefe.de:/cvs -z9 co dietlibc libowfat gatling
dietlibc is not strictly needed, but it's my environment.
First built dietlibc, then libowfat, then gatling.
Felix
^ permalink raw reply
* Re: bizarre network timing problem
From: Rick Jones @ 2007-11-02 22:58 UTC (permalink / raw)
To: Felix von Leitner; +Cc: Chuck Ebbert, linux-kernel, Netdev
In-Reply-To: <20071102223856.GA5121@codeblau.de>
Felix von Leitner wrote:
> Thus spake Rick Jones (rick.jones2@hp.com):
>
>>>Oh I'm pretty sure it's not my application, because my application performs
>>>well over ethernet, which is after all its purpose. Also I see the
>>>write, the TCP uncork, then a pause, and then the packet leaving.
>>
>>Well, a wise old engineer tried to teach me that the proper spelling is
>>ass-u-me :) so just for grins, you might try the TCP_RR test anyway :) And
>>even if your application is correct (although I wonder why the receiver
>>isn't sucking data-out very quickly...) if you can reproduce the problem
>>with netperf it will be easier for others to do so.
>
>
> My application is only the server, the receiver is smbget from Samba, so
> I don't feel responsible for it :-)
Might want to strace it anyway... no good deed (such as reporting a potential
issue) goes unpunished :)
> Still, when run over Ethernet, it works fine without waiting for
> timeouts to expire.
Past performance is no guarantee of current correctness :) And over an
Ethernet, there will be a very different set of both timings and TCP segment
sizes compared to loopback.
My guess is that you will find setting the lo mtu to 1500 a very interesting
experiment.
>
> To reproduce this:
>
> - smbget is from samba, you probably already have this
> - gatling (my server) can be gotten from
> cvs -d :pserver:cvs@cvs.fefe.de:/cvs -z9 co dietlibc libowfat gatling
>
> dietlibc is not strictly needed, but it's my environment.
> First built dietlibc, then libowfat, then gatling.
>
> Felix
^ permalink raw reply
* [PATCH 1/1]: Using ICMP type and code in xfrm selector
From: Joy Latten @ 2007-11-02 23:17 UTC (permalink / raw)
To: netdev; +Cc: davem, herbert, tgraf
ICMP message type and/or code may be value "0" when
used as selector.
Currently, if you specify SPD entry with upper layer
protocol set as icmp, specify message type is 0 and
code is 0, then all icmp messages get
mapped to this. It appears value 0 for port is
interpreted to mean ANY, which is not entirely true for
ICMP and MH message type values.
Below patch fixes this so ICMP message types and
codes as well as MH types having value "0" aren't
interpreted as ANY.
While fixing this I wondered why we put icmp message
type in sport and code in dport?
recent ipsec rfc 4301 says:
If the Next Layer Protocol is a Mobility Header, then there
is a selector for IPv6 Mobility Header message type (MH type)
[Mobip]. This is an 8-bit value that identifies a particular
mobility message. Note that the MH type may not be available
in the case of receipt of a fragmented packet. (See Section
7, "Handling Fragments".) For IKE, the IPv6 Mobility Header
message type (MH type) is placed in the most significant
eight bits of the 16-bit local "port" selector.
If the Next Layer Protocol value is ICMP, then there is a
16-bit selector for the ICMP message type and code. The
message type is a single 8-bit value, which defines the type
of an ICMP message, or ANY. The ICMP code is a single 8-bit
value that defines a specific subtype for an ICMP message.
For IKE, the message type is placed in the most significant 8
bits of the 16-bit selector and the code is placed in the
least significant 8 bits.
Should I leave as is or put both type and code into sport
and also copy into dport to be closer to rfc? Similar
question for MH type...
Seems ok as is, but I could be missing something.
xfrm_user did not appear to require this change.
I tested icmp with my patched ipsec-tools.
Signed-off-by: Joy Latten <latten@austin.ibm.com>
diff -urpN linux-2.6.24-rc1-git11/include/linux/ipsec.h linux-2.6.24-rc1-git11.patch/include/linux/ipsec.h
--- linux-2.6.24-rc1-git11/include/linux/ipsec.h 2007-11-02 16:36:30.000000000 -0500
+++ linux-2.6.24-rc1-git11.patch/include/linux/ipsec.h 2007-11-02 16:52:57.000000000 -0500
@@ -8,6 +8,7 @@
#define IPSEC_PORT_ANY 0
#define IPSEC_ULPROTO_ANY 255
#define IPSEC_PROTO_ANY 255
+#define IPSEC_ICMPMH_ANY 255
enum {
IPSEC_MODE_ANY = 0, /* We do not support this for SA */
diff -urpN linux-2.6.24-rc1-git11/net/key/af_key.c linux-2.6.24-rc1-git11.patch/net/key/af_key.c
--- linux-2.6.24-rc1-git11/net/key/af_key.c 2007-11-02 16:39:40.000000000 -0500
+++ linux-2.6.24-rc1-git11.patch/net/key/af_key.c 2007-11-02 16:44:17.000000000 -0500
@@ -568,6 +568,35 @@ static int pfkey_sadb_addr2xfrm_addr(str
/* NOTREACHED */
}
+static void pfkey_set_sportmask(struct xfrm_selector *sel)
+{
+ switch(sel->proto) {
+ case IPPROTO_ICMP:
+ case IPPROTO_ICMPV6:
+ case IPPROTO_MH:
+ if (sel->sport != IPSEC_ICMPMH_ANY)
+ sel->sport_mask = htons(0xffff);
+ break;
+ default:
+ if (sel->sport)
+ sel->sport_mask = htons(0xffff);
+ }
+}
+
+static void pfkey_set_dportmask(struct xfrm_selector *sel)
+{
+ switch(sel->proto) {
+ case IPPROTO_ICMP:
+ case IPPROTO_ICMPV6:
+ if (sel->dport != IPSEC_ICMPMH_ANY)
+ sel->dport_mask = htons(0xffff);
+ break;
+ default:
+ if (sel->dport)
+ sel->dport_mask = htons(0xffff);
+ }
+}
+
static struct xfrm_state *pfkey_xfrm_state_lookup(struct sadb_msg *hdr, void **ext_hdrs)
{
struct sadb_sa *sa;
@@ -2218,8 +2247,7 @@ static int pfkey_spdadd(struct sock *sk,
xp->selector.prefixlen_s = sa->sadb_address_prefixlen;
xp->selector.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
xp->selector.sport = ((struct sockaddr_in *)(sa+1))->sin_port;
- if (xp->selector.sport)
- xp->selector.sport_mask = htons(0xffff);
+ pfkey_set_sportmask(&xp->selector);
sa = ext_hdrs[SADB_EXT_ADDRESS_DST-1],
pfkey_sadb_addr2xfrm_addr(sa, &xp->selector.daddr);
@@ -2231,8 +2259,7 @@ static int pfkey_spdadd(struct sock *sk,
xp->selector.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
xp->selector.dport = ((struct sockaddr_in *)(sa+1))->sin_port;
- if (xp->selector.dport)
- xp->selector.dport_mask = htons(0xffff);
+ pfkey_set_dportmask(&xp->selector);
sec_ctx = (struct sadb_x_sec_ctx *) ext_hdrs[SADB_X_EXT_SEC_CTX-1];
if (sec_ctx != NULL) {
@@ -2324,16 +2351,14 @@ static int pfkey_spddelete(struct sock *
sel.prefixlen_s = sa->sadb_address_prefixlen;
sel.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
sel.sport = ((struct sockaddr_in *)(sa+1))->sin_port;
- if (sel.sport)
- sel.sport_mask = htons(0xffff);
+ pfkey_set_sportmask(&sel);
sa = ext_hdrs[SADB_EXT_ADDRESS_DST-1],
pfkey_sadb_addr2xfrm_addr(sa, &sel.daddr);
sel.prefixlen_d = sa->sadb_address_prefixlen;
sel.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
sel.dport = ((struct sockaddr_in *)(sa+1))->sin_port;
- if (sel.dport)
- sel.dport_mask = htons(0xffff);
+ pfkey_set_dportmask(&sel);
sec_ctx = (struct sadb_x_sec_ctx *) ext_hdrs[SADB_X_EXT_SEC_CTX-1];
memset(&tmp, 0, sizeof(struct xfrm_policy));
@@ -2548,8 +2573,7 @@ static int pfkey_migrate(struct sock *sk
sel.prefixlen_s = sa->sadb_address_prefixlen;
sel.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
sel.sport = ((struct sockaddr_in *)(sa + 1))->sin_port;
- if (sel.sport)
- sel.sport_mask = htons(0xffff);
+ pfkey_set_sportmask(&sel);
/* set destination address info of selector */
sa = ext_hdrs[SADB_EXT_ADDRESS_DST - 1],
@@ -2557,8 +2581,7 @@ static int pfkey_migrate(struct sock *sk
sel.prefixlen_d = sa->sadb_address_prefixlen;
sel.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
sel.dport = ((struct sockaddr_in *)(sa + 1))->sin_port;
- if (sel.dport)
- sel.dport_mask = htons(0xffff);
+ pfkey_set_dportmask(&sel);
rq = (struct sadb_x_ipsecrequest *)(pol + 1);
^ permalink raw reply
* Re: bizarre network timing problem
From: Felix von Leitner @ 2007-11-02 23:23 UTC (permalink / raw)
To: Rick Jones; +Cc: Chuck Ebbert, linux-kernel, Netdev
In-Reply-To: <472BAB90.8020306@hp.com>
Thus spake Rick Jones (rick.jones2@hp.com):
> Past performance is no guarantee of current correctness :) And over an
> Ethernet, there will be a very different set of both timings and TCP
> segment sizes compared to loopback.
> My guess is that you will find setting the lo mtu to 1500 a very
> interesting experiment.
Setting the MTU on lo to 1500 eliminates the problem and gives me double
digit MB/sec throughput.
Felix
^ permalink raw reply
* Re: [RFC][BNX2X] .h files rewrite
From: Max Asbock @ 2007-11-02 23:35 UTC (permalink / raw)
To: Eliezer Tamir
Cc: davem@davemloft.net, Michael Chan, jeff@garzik.org,
netdev@vger.kernel.org
In-Reply-To: <1193602670.4836.25.camel@eliezer>
On Sun, 2007-10-28 at 22:17 +0200, Eliezer Tamir wrote:
> Dave,
>
> Please take a look and tell me if this is better than the last attempt.
>
> Main changes:
>
> * Rewrite of the init code:
> 1. Full rewrite of the generated code into something easier to read.
> 2. Removed zeroes by adding a zero command.
> 3. Big chucks of data are read by the chip using the DMA engine (instead of being written by the driver).
> 4. Gzipped more of the data.
>
> * Removed BE microcode, the swapping is done in the driver instead.
>
> * Rewrite of the HSI - broke it into three files bnx2x_hsi.h bnx2x_reg.h and bnx2x_fw_defs.h
>
> * Fixed SPARSE warnings.
>
> * Fixed all but three checkpatch.pl warnings. (IMHO they are false positives)
>
> The diff is against latest net-2.6.
>
> ftp link:
> ftp://Net_sys_anon@ftp1.broadcom.com/0001-bnx2x-h-file-rewrite.txt
>
> gzipped:
> ftp://Net_sys_anon@ftp1.broadcom.com/0001-bnx2x-h-file-rewrite.txt.gz
>
> (will also attache patch inline as reply to this email for review)
>
I built the newest bnx2x code against the net-2.6 kernel and ran a
number of stress tests with netperf and pktgen. I did not encounter any
errors.
Max
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox