* [PATCH] : bug fix in multipath drr code.
From: pravin @ 2005-05-23 12:26 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Herbert Xu
[-- Attachment #1: Type: text/plain, Size: 786 bytes --]
hi
AFAIU, there is a race condition in multipath_drr code,
these code paths try to access & change last_selection variable
without any synchronization.
Please correct me if I am wrong.
Code Path - 1
__ip_route_output_key(...)
{
...
calls drr_select_route
if(FLOWI_FLAG_MULTIPATHOLDROUTE set){
check last_selection.
return last_selection //so it can return NULL pointer
}
....
}
Code Path - 2
rt_secret_rebuild()
{
.....
rt_run_flush(..);
->rt_free(rth);
-> multipath_remove(rt)
->drr_remove()
reset last_selection.
....
}
Attached patch also fixes bug in function drr_init ::
multipath_alg_register(..) is called with wrong algorithm ID.
Regards
Pravin.
[-- Attachment #2: multipath-last_selection-race-fix.patch --]
[-- Type: text/x-patch, Size: 1913 bytes --]
Signed-off by: Pravin B. Shelar <pravins@calsoftinc.com>
Index: linux-2.6.12-rc4/net/ipv4/multipath_drr.c
===================================================================
--- linux-2.6.12-rc4.orig/net/ipv4/multipath_drr.c 2005-05-06 22:20:31.000000000 -0700
+++ linux-2.6.12-rc4/net/ipv4/multipath_drr.c 2005-05-22 06:41:39.000000000 -0700
@@ -145,11 +145,13 @@
int cur_min_devidx = -1;
/* if necessary and possible utilize the old alternative */
- if ((flp->flags & FLOWI_FLAG_MULTIPATHOLDROUTE) != 0 &&
- last_selection != NULL) {
- result = last_selection;
- *rp = result;
- return;
+ if ((flp->flags & FLOWI_FLAG_MULTIPATHOLDROUTE) != 0 ) {
+ struct rtable *last_result = last_selection;
+ if(last_result != NULL &&
+ multipath_comparekeys(&last_result->fl, flp)) {
+ *rp = last_result;
+ return;
+ }
}
/* 1. make sure all alt. nexthops have the same GC related data */
@@ -244,7 +246,7 @@
if (err)
return err;
- err = multipath_alg_register(&drr_ops, IP_MP_ALG_RR);
+ err = multipath_alg_register(&drr_ops, IP_MP_ALG_DRR);
if (err)
goto fail;
Index: linux-2.6.12-rc4/net/ipv4/multipath_rr.c
===================================================================
--- linux-2.6.12-rc4.orig/net/ipv4/multipath_rr.c 2005-05-06 22:20:31.000000000 -0700
+++ linux-2.6.12-rc4/net/ipv4/multipath_rr.c 2005-05-22 06:41:20.000000000 -0700
@@ -64,10 +64,13 @@
int min_use = -1;
/* if necessary and possible utilize the old alternative */
- if ((flp->flags & FLOWI_FLAG_MULTIPATHOLDROUTE) != 0 &&
- last_used != NULL) {
- result = last_used;
- goto out;
+ if ((flp->flags & FLOWI_FLAG_MULTIPATHOLDROUTE) != 0 ) {
+ struct rtable *last_result = last_used;
+ if(last_result != NULL &&
+ multipath_comparekeys(&last_result->fl, flp)) {
+ result = last_result;
+ goto out;
+ }
}
/* 1. make sure all alt. nexthops have the same GC related data
^ permalink raw reply
* Re: [PATCH] Super TSO v2
From: Herbert Xu @ 2005-05-23 9:54 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
In-Reply-To: <20050520.115154.48807299.davem@davemloft.net>
On Fri, May 20, 2005 at 06:51:54PM +0000, David S. Miller wrote:
>
> Ok, new version of the patch. Besides some cleanups,
> the main change is packet send deferral at ACK arrival
> time.
Thanks for the great work Dave.
> @@ -712,7 +713,7 @@ new_segment:
> if (!(psize -= copy))
> goto out;
>
> - if (skb->len != mss_now || (flags & MSG_OOB))
> + if (skb->len != size_goal || (flags & MSG_OOB))
> continue;
> +unsigned int tcp_current_mss(struct sock *sk, int large_allowed)
> +
> + if (tp->xmit_size_cache != xmit_cache) {
> + u16 xmit_size_goal = mss_now;
> +
> + if (doing_tso) {
> + xmit_size_goal = 65535 -
> + tp->af_specific->net_header_len -
> + tp->ext_header_len - tp->tcp_header_len;
> +
> + if (tp->rx_opt.eff_sacks)
> + xmit_size_goal -= (TCPOLEN_SACK_BASE_ALIGNED +
> + (tp->rx_opt.eff_sacks *
> + TCPOLEN_SACK_PERBLOCK));
> +
> + xmit_size_goal -= (xmit_size_goal % mss_now);
> + }
> + tp->xmit_size_goal = xmit_size_goal;
> + tp->xmit_size_cache = xmit_cache;
> + }
> +
> return mss_now;
> }
Perhaps we should set the goal based on the cwnd as we do now?
As it is I believe we may be doing more work compared to the
status quo because it's always building 64K blocks and then
cutting them up as dictated by the cwnd. On the other hand, the
current code simply builds the packets up to the size that cwnd
allows.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [PATCH] Implement NETDEV_FEAT_CHANGE
From: Catalin(ux aka Dino) BOIE @ 2005-05-23 9:43 UTC (permalink / raw)
To: netdev; +Cc: davem
[-- Attachment #1: Type: TEXT/PLAIN, Size: 278 bytes --]
Hello!
Bridge/bonding/etc. will use it (for bridge the patch
will follow today).
Patch was tested.
David, please apply.
Thank you very much.
Signed-off-by: Catalin BOIE <catab at umbrella.ro>
---
Catalin(ux aka Dino) BOIE
catab at deuroconsult.ro
http://kernel.umbrella.ro/
[-- Attachment #2: Type: TEXT/PLAIN, Size: 2706 bytes --]
--- notify1/include/linux/notifier.h 2005-03-02 09:37:48.000000000 +0200
+++ linux/include/linux/notifier.h 2005-05-20 13:17:42.000000000 +0300
@@ -56,6 +56,7 @@ extern int notifier_call_chain(struct no
#define NETDEV_CHANGEADDR 0x0008
#define NETDEV_GOING_DOWN 0x0009
#define NETDEV_CHANGENAME 0x000A
+#define NETDEV_FEAT_CHANGE 0x000B
#define SYS_DOWN 0x0001 /* Notify of system down */
#define SYS_RESTART SYS_DOWN
--- notify1/net/core/ethtool.c 2005-03-02 09:38:37.000000000 +0200
+++ linux/net/core/ethtool.c 2005-05-23 11:30:47.000000000 +0300
@@ -682,6 +682,7 @@ int dev_ethtool(struct ifreq *ifr)
void __user *useraddr = ifr->ifr_data;
u32 ethcmd;
int rc;
+ int old_features;
/*
* XXX: This can be pushed down into the ethtool_* handlers that
@@ -703,6 +704,8 @@ int dev_ethtool(struct ifreq *ifr)
if ((rc = dev->ethtool_ops->begin(dev)) < 0)
return rc;
+ old_features = dev->features;
+
switch (ethcmd) {
case ETHTOOL_GSET:
rc = ethtool_get_settings(dev, useraddr);
@@ -712,7 +715,6 @@ int dev_ethtool(struct ifreq *ifr)
break;
case ETHTOOL_GDRVINFO:
rc = ethtool_get_drvinfo(dev, useraddr);
-
break;
case ETHTOOL_GREGS:
rc = ethtool_get_regs(dev, useraddr);
@@ -801,6 +803,10 @@ int dev_ethtool(struct ifreq *ifr)
if(dev->ethtool_ops->complete)
dev->ethtool_ops->complete(dev);
+
+ if (old_features != dev->features)
+ netdev_features_change(dev);
+
return rc;
ioctl:
--- notify1/net/core/dev.c 2005-03-26 05:28:21.000000000 +0200
+++ linux/net/core/dev.c 2005-05-23 12:14:32.000000000 +0300
@@ -761,6 +761,18 @@ int dev_change_name(struct net_device *d
}
/**
+ * netdev_features_change - device changes fatures
+ * @dev: device to cause notification
+ *
+ * Called to indicate a device has changed features.
+ */
+void netdev_features_change(struct net_device *dev)
+{
+ notifier_call_chain(&netdev_chain, NETDEV_FEAT_CHANGE, dev);
+}
+EXPORT_SYMBOL(netdev_features_change);
+
+/**
* netdev_state_change - device changes state
* @dev: device to cause notification
*
--- notify1/include/linux/netdevice.h 2005-03-02 09:38:26.000000000 +0200
+++ linux/include/linux/netdevice.h 2005-05-23 10:16:43.000000000 +0300
@@ -916,6 +916,7 @@ extern void dev_mc_discard(struct net_d
extern void dev_set_promiscuity(struct net_device *dev, int inc);
extern void dev_set_allmulti(struct net_device *dev, int inc);
extern void netdev_state_change(struct net_device *dev);
+extern void netdev_features_change(struct net_device *dev);
/* Load a device via the kmod */
extern void dev_load(const char *name);
extern void dev_mcast_init(void);
^ permalink raw reply
* Re: [PATCH] [BRIDGE] Set features based on slave's ones (was Ethernet Bridging: Enable Hardware Checksumming)
From: Catalin(ux aka Dino) BOIE @ 2005-05-23 9:40 UTC (permalink / raw)
To: Jon Mason; +Cc: netdev, davem
In-Reply-To: <200505201354.46824.jdmason@us.ibm.com>
> Let's use the full BR_FEAT_MASK of NETIF_F_HW_CSUM | NETIF_F_SG |
> | NETIF_F_FRAGLIST | NETIF_F_IP_CSUM | NETIF_F_HIGHDMA | NETIF_F_TSO).
> That is 0011 0000 0110 1011
>
> Now, let's assume that the NIC only has NETIF_F_TSO,
> NETIF_F_IP_CSUM, and NETIF_F_SG enabled.
> So, that is 0000 1000 0000 0011
>
> So, we have the following:
> For the first adapter added:
> br = 0001 1000 0110 1011
> br = 0001 1000 0110 1011 & 0000 1000 0000 0011 | ~0001 1000 0110 1011
> br = 0001 1000 0110 1011 & 0000 1000 0000 0011 | 1110 0111 1001 0100
> br = 0000 1000 0000 0011 | 1110 0111 1001 0100
> br = 1110 1111 1001 0111
> Note - this breaks down to the following being enabled:
> NETIF_F_SG
> NETIF_F_IP_CSUM
> NETIF_F_NO_CSUM
> UNDEFINED BIT
> NETIF_F_HW_VLAN_RX
> NETIF_F_HW_VLAN_FILTER
> NETIF_F_VLAN_CHALLENGED
> NETIF_F_TSO
> NETIF_F_LLTX
[...]
> Thanks,
> Jon
The code applies the ~ first, then the | and _then_ the &.
If we have:
a &= a | ~b;
it is equivalent with:
a = a & (a | ~b);
For your example:
br = 0001 1000 0110 1011 = 0x186B
feat = 0001 1000 0110 1011 = 0x186B
nic = 0000 1000 0000 0011 = 0x0803
br &= nic | ~feat
br = 0x803 - I say this is correct!
With the second nic:
br = 0x0803
feat = 0x186B
nic2 = 0x0803
br &= nic2 | ~feat;
br = 0x0803 - I say it's correct
Am I missing something?
---
Catalin(ux aka Dino) BOIE
catab at deuroconsult.ro
http://kernel.umbrella.ro/
^ permalink raw reply
* Re: [PATCH] [BRIDGE] Set features based on slave's ones (was Ethernet Bridging: Enable Hardware Checksumming)
From: Catalin(ux aka Dino) BOIE @ 2005-05-23 9:39 UTC (permalink / raw)
To: Jon Mason; +Cc: netdev, davem, Stephen Hemminger
In-Reply-To: <200505201354.46824.jdmason@us.ibm.com>
>>>> +/*
>>>> + * Recomputes features using slave's features
>>>> + */
>>>> +static void br_features_recompute(struct net_bridge *br)
>>>> +{
>>>> + struct net_bridge_port *p;
>>>> +
>>>> + br->dev->features |= BR_FEAT_MASK;
>>>
>>> The OR is not needed. Just re-initialize features to BR_FEAT_MASK and
>>> everything will take case of itself.
>>
>> Nope. Not correct. Because we might enable LLTX on the bridge, but not in
>> BR_FEAT_MASK.
>
> This is not the case. This is the same as re-doing the entire feature list
> (as if it was being done initially). All that needs to be done is for
> br->dev->features to have a initial value.
>
>> I will post a patch in few hours with all stuff updated.
>
> Great. Thanks.
>
> Thanks,
> Jon
I will post a patch with the initial features as:
dev->features = BR_FEAT_MASK | NETIF_F_NO_CSUM | NETIF_F_LLTX
at Stephen's suggestion.
But, BR_FEAT_MASK will not have NETIF_F_NO_CSUM and NETIF_F_LLTX.
I think the code is correct as it is.
---
Catalin(ux aka Dino) BOIE
catab at deuroconsult.ro
http://kernel.umbrella.ro/
^ permalink raw reply
* Why /sys/class/net/*/features is read-only?
From: Catalin(ux aka Dino) BOIE @ 2005-05-23 7:46 UTC (permalink / raw)
To: netdev
Hello!
Why features file is read-only. If an admin want to force some attributes,
we can let them, right?
This way, an admin can force SG and HW_CSUM on a bridge, for example. ;)
I can make a patch, if you want.
Thank you!
---
Catalin(ux aka Dino) BOIE
catab at deuroconsult.ro
http://kernel.umbrella.ro/
^ permalink raw reply
* Re: [PATCH 11/12] orinoco: monitor mode support
From: Pavel Roskin @ 2005-05-23 3:40 UTC (permalink / raw)
To: Francois Romieu; +Cc: Christoph Hellwig, jgarzik, hermes, netdev
In-Reply-To: <20050517212217.GB12936@electric-eye.fr.zoreil.com>
Hello, Francois!
On Tue, 2005-05-17 at 23:22 +0200, Francois Romieu wrote:
> I am not fond of unneeded/hidden state variable. What about:
>
> + /* sanity check the length */
> + if (datalen > IEEE80211_DATA_LEN + 12) {
> + printk(KERN_DEBUG "%s: oversized monitor frame, "
> + "data length = %d\n", dev->name, datalen);
> + err = -EIO;
> + goto drop;
> ^^^^^^^^^^ -> let's replace by 'goto update_stats;'
> And turn:
> + drop:
> + stats->rx_errors++;
> + stats->rx_dropped++;
>
> into:
>
> + drop:
> + dev_kfree_skb_irq(skb);
> + update_stats:
> + stats->rx_errors++;
> + stats->rx_dropped++;
>
> This way 'goto drop' really drops and the code does not issue a 'goto drop'
> when it actually want to update the stats.
I agree. Closer look shows that orinoco needs a complete overhaul of
stats in the rx path. I'm going to fix it in CVS, and I'll post the
patch to netdev after it receives some testing.
--
Regards,
Pavel Roskin
^ permalink raw reply
* Re: Good (exemplary) network driver?
From: Francois Romieu @ 2005-05-22 22:34 UTC (permalink / raw)
To: Michael Renzmann; +Cc: netdev
In-Reply-To: <4290D309.4020301@otaku42.de>
Michael Renzmann <netdev@nospam.otaku42.de> :
[...]
> Short question: which network driver is commonly regarded as a good
> implementation of what a linux network driver should look like? Is there
> something like a "reference driver" available, which could be consulted
> for all of those "how should I do xyz" questions?
drivers/net/pci-skeleton.c provides some hints but it is not the best
maintained/cleaned-up/feature-rich driver (no module_parm, no ethtool, no
vlan, etc.). You will find some answers in the gigabit drivers as well.
--
Ueimor
^ permalink raw reply
* Good (exemplary) network driver?
From: Michael Renzmann @ 2005-05-22 18:44 UTC (permalink / raw)
To: netdev
Hi all.
Short question: which network driver is commonly regarded as a good
implementation of what a linux network driver should look like? Is there
something like a "reference driver" available, which could be consulted
for all of those "how should I do xyz" questions?
Bye, Mike
^ permalink raw reply
* Re: [BUG 2.6.12-rc4] IPv6 xfrm tunnel oops with large packets
From: Herbert Xu @ 2005-05-22 7:34 UTC (permalink / raw)
To: YOSHIFUJI Hideaki / ?$B5HF#1QL@; +Cc: christophe, linux-net, netdev, davem
In-Reply-To: <20050522.051817.122678688.yoshfuji@linux-ipv6.org>
On Sun, May 22, 2005 at 05:18:17AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ wrote:
>
> > BTW, icmpv6_send seems to ignore its dev argument altogether.
> > Any reason why we can't just use it instead of skb->dev?
>
> (After looking into icmpv6_send() usages,)
> I don't know the reason why we needed the last argument "dev".
> It seems that we already had it at the time of 2.2.16.
> Probably, we can remove it.
In a number of places (I've just created another one with my patch :)
we are setting skb->dev specifically for icmpv6_send. If we make
icmpv6_send use its dev argument instead of skb->dev then we can
avoid those settings.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* user space checksum
From: raz ben jehuda @ 2005-05-22 6:51 UTC (permalink / raw)
To: netdev
I do not know if this was ever suggested.so i'll try any way.
Why not give the user the chance to supply the kernel/card with the
checksums instead of having the kernel/network card do the work ?
This is because a user is aware to the data he sends and he can optimize
the data checksum calculations more than anyone else. for instance a web
server sends its html pages, which are static data. it can pre-calculate
the checksums.
Anyone ?
--
Raz
Long Live The Penguin
^ permalink raw reply
* Re: patch tulip-natsemi-dp83840a-phy-fix.patch added to -mm tree
From: Grant Grundler @ 2005-05-22 4:56 UTC (permalink / raw)
To: Francois Romieu
Cc: Jeff Garzik, Grant Grundler, akpm, T-Bone, varenet, Linux Kernel,
Netdev
In-Reply-To: <20050521223959.GA4337@electric-eye.fr.zoreil.com>
On Sun, May 22, 2005 at 12:39:59AM +0200, Francois Romieu wrote:
> Jeff Garzik <jgarzik@pobox.com> :
> [tulip_media_select]
> > 1) called from timer context, from the media poll timer
> >
> > 2) called from spin_lock_irqsave() context, in the ->tx_timeout hook.
> >
> > The first case can be fixed by moved all the timer code to a workqueue.
> > Then when the existing timer fires, kick the workqueue.
> >
> > The second case can be fixed by kicking the workqueue upon tx_timeout
> > (which is the reason why I did not suggest queue_delayed_work() use).
>
> First try below. It only moves tulip_select_media() to process context.
Cool - thanks.
> The original patch (with s/udelay/msleep/ or such) is not included.
That's fine. I'll take care of that once Jeff is happy with this.
> Comments/suggestions ?
Basic workqueue create/destroy looks correct - but I've only played with
workqueues once before.
It wouldn't hurt if someone else double checked too.
Comments below are mostly about the other parts.
> +static inline int tulip_create_workqueue(void)
> +{
> + ktulipd_workqueue = create_workqueue("ktulipd");
> + return ktulipd_workqueue ? 0 : -ENOMEM;
> +}
This just obfuscates the code. It's only called in one place.
Please just directly call create_workqueue("ktulipd") from tulip_init()
and check the return value.
> +static inline void tulip_destroy_workqueue(void)
> +{
> + destroy_workqueue(ktulipd_workqueue);
> +}
Same thing.
> @@ -526,20 +549,9 @@ static void tulip_tx_timeout(struct net_
...
> + tp->timeout_recovery = 1;
> + queue_work(ktulipd_workqueue, &tp->media_work);
> + goto out_unlock;
This is the key bit.
> - /* Stop and restart the chip's Tx processes . */
> -
> - tulip_restart_rxtx(tp);
> - /* Trigger an immediate transmit demand. */
> - iowrite32(0, ioaddr + CSR1);
> -
> - tp->stats.tx_errors++;
> + tulip_tx_timeout_complete(tp, ioaddr);
This doesn't fix the existing issue with tulip_restart_rxtx().
Even without the patch to tulip_select_media(),
tulip_restart_rxtx() does not comply with jgarzik's linux driver
requirements becuase it can spin delay up to 1200us.
> static void __exit tulip_cleanup (void)
> {
> pci_unregister_driver (&tulip_driver);
> + tulip_destroy_workqueue();
> }
Only one workqueue for all instances of tulip cards, right?
...
> @@ -127,6 +128,14 @@ void tulip_timer(unsigned long data)
> }
> break;
> }
> +
> + spin_lock_irqsave (&tp->lock, flags);
> + if (tp->timeout_recovery) {
> + tp->timeout_recovery = 0;
> + tulip_tx_timeout_complete(tp, ioaddr);
> + }
> + spin_unlock_irqrestore (&tp->lock, flags);
This suffers the original issue: blocked IRQs while CPU might spin
for 1200us in tulip_tx_timeout_complete().
If tp->timeout_recovery acts as a sort of semaphore for us,
do we even need the spinlock?
I suspect "yes" because timeout_recovery is a bitfield and clearing
it is a read/modify/write operation. This is why I don't like bitfields.
ie. something like:
if (tp->timeout_recovery) {
tulip_tx_timeout_complete(tp, ioaddr);
spin_lock_irqsave (&tp->lock, flags);
tp->timeout_recovery = 0; /* Bitfields are NOT atomic. */
spin_unlock_irqrestore (&tp->lock, flags);
}
thanks,
grant
^ permalink raw reply
* Re: Network driver behaviour when loosing link
From: Tommy Christensen @ 2005-05-22 0:52 UTC (permalink / raw)
To: Michael Renzmann; +Cc: netdev
In-Reply-To: <428F949C.8080707@otaku42.de>
Michael Renzmann wrote:
> Hi all.
>
> I'm playing with a network driver and came across one thing I'd like to
> ask something about.
>
> Let's assume a driver detects a link status change (for example, the
> link is dropped). I would have expected that the driver should signal
> the change via netif_carrier_on/_off (if used at all) and tell the
> kernel to stop/resume sending packets to the driver (via
> netif_wake_queue/netif_stop_queue).
>
> When checking several drivers I found some that call
> netif_[wake|stop]_queue (e100, e1000), while others don't (natsemi, tg3).
>
> Hence the question: what is the recommended behaviour in such a case? Is
netif_carrier_on/off should certainly be used.
The use of netif_stop_queue depends on your HW:
A) If your NIC continues taking packets from the ring buffer while link
is down, then just let it do so. No need to call netif_stop_queue.
Since recently, the stack will seize sending packets to your driver
anyway. NB: this doesn't happen instantly, but typically within 1 sec.
after calling netif_carrier_off.
B) If your NIC stops DMA'ing packets when link is down, you have two
options:
1) Check netif_carrier_ok in your hard_start_xmit function.
2) Call netif_stop_queue when link is lost.
In both cases you have to flush the NIC's TX ring buffer, in order not
to hold up resources (and not to send out stale packets when link is
restored).
> it ok to call netif_[stop|wake]_queue? How about adjusting the
The queue state is assumed stable when holding dev->xmit_lock, so you can
only call netif_stop_queue/wake_queue when this lock is held. However,
this requirement doesn't seem do be followed everywhere. And then there
is the LLTX drivers...
> IFF_RUNNING flag accordingly? Is there any documentation available on
> this topic?
Drivers shouldn't touch IFF_RUNNING. It is maintained by the stack.
Documentation? I haven't really seen any myself, except for the usual:
source code, and mailing-list discussions. Any pointers are welcome'd.
-Tommy
^ permalink raw reply
* Re: [Netem] [PATCH] (3/3) netem: allow random reordering
From: Julio Kriger @ 2005-05-21 23:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
In-Reply-To: <20050519151254.79afe7e7@dxpl.pdx.osdl.net>
Hi,
I have a doubt about about this lines (#90)
if (q->gap == 0 && q->counter < q->gap &&
q->reorder < get_crandom(&q->reorder_cor)) {
I could not achieve reordering. I've set gap and reordering values but
did'nt success. Then I've modified those lines to this one
if( 0 < q->reorder && q->reorder < get_crandom(&q->reorder_cor) ) {
and I achieve reordering (I test it pinging localhost).
Is this correct or I'm doing something wrong?
Regards,
Julio
On 5/19/05, Stephen Hemminger <shemminger@osdl.org> wrote:
> Enhance the reorder feature of netem to allow random percent to be reordered.
> Has expected backwards compatibility behaviour.
>
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
>
> Index: netem-2.6.12-rc4/include/linux/pkt_sched.h
> ===================================================================
> --- netem-2.6.12-rc4.orig/include/linux/pkt_sched.h
> +++ netem-2.6.12-rc4/include/linux/pkt_sched.h
> @@ -427,6 +427,7 @@ enum
> TCA_NETEM_UNSPEC,
> TCA_NETEM_CORR,
> TCA_NETEM_DELAY_DIST,
> + TCA_NETEM_REORDER,
> __TCA_NETEM_MAX,
> };
>
> @@ -437,7 +438,7 @@ struct tc_netem_qopt
> __u32 latency; /* added delay (us) */
> __u32 limit; /* fifo limit (packets) */
> __u32 loss; /* random packet loss (0=none ~0=100%) */
> - __u32 gap; /* re-ordering gap (0 for delay all) */
> + __u32 gap; /* re-ordering gap (0 for none) */
> __u32 duplicate; /* random packet dup (0=none ~0=100%) */
> __u32 jitter; /* random jitter in latency (us) */
> };
> @@ -449,6 +450,12 @@ struct tc_netem_corr
> __u32 dup_corr; /* duplicate correlation */
> };
>
> +struct tc_netem_reorder
> +{
> + __u32 probability;
> + __u32 correlation;
> +};
> +
> #define NETEM_DIST_SCALE 8192
>
> #endif
> Index: netem-2.6.12-rc4/net/sched/sch_netem.c
> ===================================================================
> --- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
> +++ netem-2.6.12-rc4/net/sched/sch_netem.c
> @@ -62,11 +62,12 @@ struct netem_sched_data {
> u32 gap;
> u32 jitter;
> u32 duplicate;
> + u32 reorder;
>
> struct crndstate {
> unsigned long last;
> unsigned long rho;
> - } delay_cor, loss_cor, dup_cor;
> + } delay_cor, loss_cor, dup_cor, reorder_cor;
>
> struct disttable {
> u32 size;
> @@ -185,18 +186,18 @@ static int netem_enqueue(struct sk_buff
> * of the queue.
> * gap == 0 is special case for no-reordering.
> */
> - if (q->gap == 0 || q->counter != q->gap) {
> + if (q->gap == 0 && q->counter < q->gap &&
> + q->reorder < get_crandom(&q->reorder_cor)) {
> psched_time_t now;
> PSCHED_GET_TIME(now);
> - PSCHED_TADD2(now,
> - tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
> + PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
> + &q->delay_cor, q->delay_dist),
> cb->time_to_send);
> -
> ++q->counter;
> ret = q->qdisc->enqueue(skb, q->qdisc);
> } else {
> - q->counter = 0;
> PSCHED_GET_TIME(cb->time_to_send);
> + q->counter = 0;
> ret = q->qdisc->ops->requeue(skb, q->qdisc);
> }
>
> @@ -351,6 +352,19 @@ static int get_correlation(struct Qdisc
> return 0;
> }
>
> +static int get_reorder(struct Qdisc *sch, const struct rtattr *attr)
> +{
> + struct netem_sched_data *q = qdisc_priv(sch);
> + const struct tc_netem_reorder *r = RTA_DATA(attr);
> +
> + if (RTA_PAYLOAD(attr) != sizeof(*r))
> + return -EINVAL;
> +
> + q->reorder = r->probability;
> + init_crandom(&q->reorder_cor, r->correlation);
> + return 0;
> +}
> +
> static int netem_change(struct Qdisc *sch, struct rtattr *opt)
> {
> struct netem_sched_data *q = qdisc_priv(sch);
> @@ -371,9 +385,15 @@ static int netem_change(struct Qdisc *sc
> q->jitter = qopt->jitter;
> q->limit = qopt->limit;
> q->gap = qopt->gap;
> + q->counter = 0;
> q->loss = qopt->loss;
> q->duplicate = qopt->duplicate;
>
> + /* for compatiablity with earlier versions.
> + * if gap is set, need to assume 100% probablity
> + */
> + q->reorder = ~0;
> +
> /* Handle nested options after initial queue options.
> * Should have put all options in nested format but too late now.
> */
> @@ -395,6 +415,11 @@ static int netem_change(struct Qdisc *sc
> if (ret)
> return ret;
> }
> + if (tb[TCA_NETEM_REORDER-1]) {
> + ret = get_reorder(sch, tb[TCA_NETEM_REORDER-1]);
> + if (ret)
> + return ret;
> + }
> }
>
>
> @@ -412,7 +437,6 @@ static int netem_init(struct Qdisc *sch,
> init_timer(&q->timer);
> q->timer.function = netem_watchdog;
> q->timer.data = (unsigned long) sch;
> - q->counter = 0;
>
> q->qdisc = qdisc_create_dflt(sch->dev, &pfifo_qdisc_ops);
> if (!q->qdisc) {
> @@ -444,6 +468,7 @@ static int netem_dump(struct Qdisc *sch,
> struct rtattr *rta = (struct rtattr *) b;
> struct tc_netem_qopt qopt;
> struct tc_netem_corr cor;
> + struct tc_netem_reorder reorder;
>
> qopt.latency = q->latency;
> qopt.jitter = q->jitter;
> @@ -457,6 +482,11 @@ static int netem_dump(struct Qdisc *sch,
> cor.loss_corr = q->loss_cor.rho;
> cor.dup_corr = q->dup_cor.rho;
> RTA_PUT(skb, TCA_NETEM_CORR, sizeof(cor), &cor);
> +
> + reorder.probability = q->reorder;
> + reorder.correlation = q->reorder_cor.rho;
> + RTA_PUT(skb, TCA_NETEM_REORDER, sizeof(reorder), &reorder);
> +
> rta->rta_len = skb->tail - b;
>
> return skb->len;
>
>
> _______________________________________________
> Netem mailing list
> Netem@lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/netem
>
>
>
--
----------------------------
Julio Kriger
mailto:juliokriger@gmail.com
^ permalink raw reply
* Re: patch tulip-natsemi-dp83840a-phy-fix.patch added to -mm tree
From: Francois Romieu @ 2005-05-21 22:39 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Grant Grundler, akpm, T-Bone, varenet, Linux Kernel, Netdev
In-Reply-To: <4288CE51.1050703@pobox.com>
Jeff Garzik <jgarzik@pobox.com> :
[tulip_media_select]
> 1) called from timer context, from the media poll timer
>
> 2) called from spin_lock_irqsave() context, in the ->tx_timeout hook.
>
> The first case can be fixed by moved all the timer code to a workqueue.
> Then when the existing timer fires, kick the workqueue.
>
> The second case can be fixed by kicking the workqueue upon tx_timeout
> (which is the reason why I did not suggest queue_delayed_work() use).
First try below. It only moves tulip_select_media() to process context.
The original patch (with s/udelay/msleep/ or such) is not included.
Patch applies/compiles against 2.6.12-rc4.
Comments/suggestions ?
diff -puN a/drivers/net/tulip/tulip.h~tulip-000 b/drivers/net/tulip/tulip.h
--- a/drivers/net/tulip/tulip.h~tulip-000 2005-05-21 22:30:08.133891956 +0200
+++ b/drivers/net/tulip/tulip.h 2005-05-22 00:18:45.598682086 +0200
@@ -46,6 +46,7 @@ struct tulip_chip_table {
int valid_intrs; /* CSR7 interrupt enable settings */
int flags;
void (*media_timer) (unsigned long data);
+ void (*media_task) (void *);
};
@@ -368,6 +369,7 @@ struct tulip_private {
unsigned int medialock:1; /* Don't sense media type. */
unsigned int mediasense:1; /* Media sensing in progress. */
unsigned int nway:1, nwayset:1; /* 21143 internal NWay. */
+ unsigned int timeout_recovery: 1;
unsigned int csr0; /* CSR0 setting. */
unsigned int csr6; /* Current CSR6 control settings. */
unsigned char eeprom[EEPROM_SIZE]; /* Serial EEPROM contents. */
@@ -386,6 +388,7 @@ struct tulip_private {
void __iomem *base_addr;
int csr12_shadow;
int pad0; /* Used for 8-byte alignment */
+ struct work_struct media_work;
};
@@ -400,7 +403,7 @@ struct eeprom_fixup {
/* 21142.c */
extern u16 t21142_csr14[];
-void t21142_timer(unsigned long data);
+void t21142_media_task(void *data);
void t21142_start_nway(struct net_device *dev);
void t21142_lnk_change(struct net_device *dev, int csr5);
@@ -438,7 +441,7 @@ void pnic_lnk_change(struct net_device *
void pnic_timer(unsigned long data);
/* timer.c */
-void tulip_timer(unsigned long data);
+void tulip_media_task(void *data);
void mxic_timer(unsigned long data);
void comet_timer(unsigned long data);
@@ -490,4 +493,13 @@ static inline void tulip_restart_rxtx(st
tulip_start_rxtx(tp);
}
+static inline void tulip_tx_timeout_complete(struct tulip_private *tp, void __iomem *ioaddr)
+{
+ /* Stop and restart the chip's Tx processes. */
+ tulip_restart_rxtx(tp);
+ /* Trigger an immediate transmit demand. */
+ iowrite32(0, ioaddr + CSR1);
+
+ tp->stats.tx_errors++;
+}
#endif /* __NET_TULIP_H__ */
diff -puN a/drivers/net/tulip/tulip_core.c~tulip-000 b/drivers/net/tulip/tulip_core.c
--- a/drivers/net/tulip/tulip_core.c~tulip-000 2005-05-21 20:57:38.385293188 +0200
+++ b/drivers/net/tulip/tulip_core.c 2005-05-22 00:24:29.144946592 +0200
@@ -132,6 +132,16 @@ int tulip_debug = 1;
#endif
+static struct workqueue_struct *ktulipd_workqueue;
+
+static void tulip_timer(unsigned long data)
+{
+ struct net_device *dev = (struct net_device *)data;
+ struct tulip_private *tp = netdev_priv(dev);
+
+ if (likely(netif_running(dev)))
+ queue_work(ktulipd_workqueue, &tp->media_work);
+}
/*
* This table use during operation for capabilities and media timer.
@@ -145,63 +155,65 @@ struct tulip_chip_table tulip_tbl[] = {
/* DC21140 */
{ "Digital DS21140 Tulip", 128, 0x0001ebef,
- HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM | HAS_PCI_MWI, tulip_timer },
+ HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM | HAS_PCI_MWI, tulip_timer,
+ tulip_media_task },
/* DC21142, DC21143 */
{ "Digital DS21143 Tulip", 128, 0x0801fbff,
HAS_MII | HAS_MEDIA_TABLE | ALWAYS_CHECK_MII | HAS_ACPI | HAS_NWAY
- | HAS_INTR_MITIGATION | HAS_PCI_MWI, t21142_timer },
+ | HAS_INTR_MITIGATION | HAS_PCI_MWI, tulip_timer, t21142_media_task },
/* LC82C168 */
{ "Lite-On 82c168 PNIC", 256, 0x0001fbef,
- HAS_MII | HAS_PNICNWAY, pnic_timer },
+ HAS_MII | HAS_PNICNWAY, pnic_timer, },
/* MX98713 */
{ "Macronix 98713 PMAC", 128, 0x0001ebef,
- HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM, mxic_timer },
+ HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM, mxic_timer, },
/* MX98715 */
{ "Macronix 98715 PMAC", 256, 0x0001ebef,
- HAS_MEDIA_TABLE, mxic_timer },
+ HAS_MEDIA_TABLE, mxic_timer, },
/* MX98725 */
{ "Macronix 98725 PMAC", 256, 0x0001ebef,
- HAS_MEDIA_TABLE, mxic_timer },
+ HAS_MEDIA_TABLE, mxic_timer, },
/* AX88140 */
{ "ASIX AX88140", 128, 0x0001fbff,
HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM | MC_HASH_ONLY
- | IS_ASIX, tulip_timer },
+ | IS_ASIX, tulip_timer, tulip_media_task },
/* PNIC2 */
{ "Lite-On PNIC-II", 256, 0x0801fbff,
- HAS_MII | HAS_NWAY | HAS_8023X | HAS_PCI_MWI, pnic2_timer },
+ HAS_MII | HAS_NWAY | HAS_8023X | HAS_PCI_MWI, pnic2_timer, },
/* COMET */
{ "ADMtek Comet", 256, 0x0001abef,
- HAS_MII | MC_HASH_ONLY | COMET_MAC_ADDR, comet_timer },
+ HAS_MII | MC_HASH_ONLY | COMET_MAC_ADDR, comet_timer, },
/* COMPEX9881 */
{ "Compex 9881 PMAC", 128, 0x0001ebef,
- HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM, mxic_timer },
+ HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM, mxic_timer, },
/* I21145 */
{ "Intel DS21145 Tulip", 128, 0x0801fbff,
HAS_MII | HAS_MEDIA_TABLE | ALWAYS_CHECK_MII | HAS_ACPI
- | HAS_NWAY | HAS_PCI_MWI, t21142_timer },
+ | HAS_NWAY | HAS_PCI_MWI, tulip_timer, t21142_media_task },
/* DM910X */
{ "Davicom DM9102/DM9102A", 128, 0x0001ebef,
HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM | HAS_ACPI,
- tulip_timer },
+ tulip_timer, tulip_media_task },
/* RS7112 */
{ "Conexant LANfinity", 256, 0x0001ebef,
- HAS_MII | HAS_ACPI, tulip_timer },
+ HAS_MII | HAS_ACPI, tulip_timer, tulip_media_task },
/* ULi526X */
{ "ULi M5261/M5263", 128, 0x0001ebef,
- HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM | HAS_ACPI, tulip_timer },
+ HAS_MII | HAS_MEDIA_TABLE | CSR12_IN_SROM | HAS_ACPI, tulip_timer,
+ tulip_media_task },
};
@@ -265,6 +277,17 @@ static void set_rx_mode(struct net_devic
static void poll_tulip(struct net_device *dev);
#endif
+static inline int tulip_create_workqueue(void)
+{
+ ktulipd_workqueue = create_workqueue("ktulipd");
+ return ktulipd_workqueue ? 0 : -ENOMEM;
+}
+
+static inline void tulip_destroy_workqueue(void)
+{
+ destroy_workqueue(ktulipd_workqueue);
+}
+
static void tulip_set_power_state (struct tulip_private *tp,
int sleep, int snooze)
{
@@ -526,20 +549,9 @@ static void tulip_tx_timeout(struct net_
"SIA %8.8x %8.8x %8.8x %8.8x, resetting...\n",
dev->name, ioread32(ioaddr + CSR5), ioread32(ioaddr + CSR12),
ioread32(ioaddr + CSR13), ioread32(ioaddr + CSR14), ioread32(ioaddr + CSR15));
- if ( ! tp->medialock && tp->mtable) {
- do
- --tp->cur_index;
- while (tp->cur_index >= 0
- && (tulip_media_cap[tp->mtable->mleaf[tp->cur_index].media]
- & MediaIsFD));
- if (--tp->cur_index < 0) {
- /* We start again, but should instead look for default. */
- tp->cur_index = tp->mtable->leafcount - 1;
- }
- tulip_select_media(dev, 0);
- printk(KERN_WARNING "%s: transmit timed out, switching to %s "
- "media.\n", dev->name, medianame[dev->if_port]);
- }
+ tp->timeout_recovery = 1;
+ queue_work(ktulipd_workqueue, &tp->media_work);
+ goto out_unlock;
} else if (tp->chip_id == PNIC2) {
printk(KERN_WARNING "%s: PNIC2 transmit timed out, status %8.8x, "
"CSR6/7 %8.8x / %8.8x CSR12 %8.8x, resetting...\n",
@@ -579,14 +591,9 @@ static void tulip_tx_timeout(struct net_
}
#endif
- /* Stop and restart the chip's Tx processes . */
-
- tulip_restart_rxtx(tp);
- /* Trigger an immediate transmit demand. */
- iowrite32(0, ioaddr + CSR1);
-
- tp->stats.tx_errors++;
+ tulip_tx_timeout_complete(tp, ioaddr);
+out_unlock:
spin_unlock_irqrestore (&tp->lock, flags);
dev->trans_start = jiffies;
netif_wake_queue (dev);
@@ -736,6 +743,8 @@ static void tulip_down (struct net_devic
void __iomem *ioaddr = tp->base_addr;
unsigned long flags;
+ flush_workqueue(ktulipd_workqueue);
+
del_timer_sync (&tp->timer);
#ifdef CONFIG_TULIP_NAPI
del_timer_sync (&tp->oom_timer);
@@ -1408,6 +1417,8 @@ static int __devinit tulip_init_one (str
tp->timer.data = (unsigned long)dev;
tp->timer.function = tulip_tbl[tp->chip_id].media_timer;
+ INIT_WORK(&tp->media_work, tulip_tbl[tp->chip_id].media_task, dev);
+
dev->base_addr = (unsigned long)ioaddr;
#ifdef CONFIG_TULIP_MWI
@@ -1838,6 +1849,8 @@ static struct pci_driver tulip_driver =
static int __init tulip_init (void)
{
+ int ret;
+
#ifdef MODULE
printk (KERN_INFO "%s", version);
#endif
@@ -1846,14 +1859,23 @@ static int __init tulip_init (void)
tulip_rx_copybreak = rx_copybreak;
tulip_max_interrupt_work = max_interrupt_work;
+ ret = tulip_create_workqueue();
+ if (ret < 0)
+ goto out;
+
/* probe for and init boards */
- return pci_module_init (&tulip_driver);
+ ret = pci_module_init(&tulip_driver);
+ if (ret < 0)
+ tulip_destroy_workqueue();
+out:
+ return ret;
}
static void __exit tulip_cleanup (void)
{
pci_unregister_driver (&tulip_driver);
+ tulip_destroy_workqueue();
}
diff -puN a/drivers/net/tulip/21142.c~tulip-000 b/drivers/net/tulip/21142.c
--- a/drivers/net/tulip/21142.c~tulip-000 2005-05-21 20:57:37.978358686 +0200
+++ b/drivers/net/tulip/21142.c 2005-05-22 00:10:21.717431845 +0200
@@ -26,9 +26,9 @@ static u16 t21142_csr15[] = { 0x0008, 0x
/* Handle the 21143 uniquely: do autoselect with NWay, not the EEPROM list
of available transceivers. */
-void t21142_timer(unsigned long data)
+void t21142_media_task(void *data)
{
- struct net_device *dev = (struct net_device *)data;
+ struct net_device *dev = data;
struct tulip_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->base_addr;
int csr12 = ioread32(ioaddr + CSR12);
diff -puN a/drivers/net/tulip/timer.c~tulip-000 b/drivers/net/tulip/timer.c
--- a/drivers/net/tulip/timer.c~tulip-000 2005-05-21 20:57:38.251314753 +0200
+++ b/drivers/net/tulip/timer.c 2005-05-22 00:24:05.889719386 +0200
@@ -18,13 +18,14 @@
#include "tulip.h"
-void tulip_timer(unsigned long data)
+void tulip_media_task(void *data)
{
- struct net_device *dev = (struct net_device *)data;
+ struct net_device *dev = data;
struct tulip_private *tp = netdev_priv(dev);
void __iomem *ioaddr = tp->base_addr;
u32 csr12 = ioread32(ioaddr + CSR12);
int next_tick = 2*HZ;
+ unsigned long flags;
if (tulip_debug > 2) {
printk(KERN_DEBUG "%s: Media selection tick, %s, status %8.8x mode"
@@ -127,6 +128,14 @@ void tulip_timer(unsigned long data)
}
break;
}
+
+ spin_lock_irqsave (&tp->lock, flags);
+ if (tp->timeout_recovery) {
+ tp->timeout_recovery = 0;
+ tulip_tx_timeout_complete(tp, ioaddr);
+ }
+ spin_unlock_irqrestore (&tp->lock, flags);
+
/* mod_timer synchronizes us with potential add_timer calls
* from interrupts.
*/
_
^ permalink raw reply
* Re: [BUG 2.6.12-rc4] IPv6 xfrm tunnel oops with large packets
From: Christophe Saout @ 2005-05-21 21:49 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-net, netdev, YOSHIFUJI Hideaki, David S. Miller
In-Reply-To: <20050521194932.GA10959@gondor.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 121 bytes --]
Am Sonntag, den 22.05.2005, 05:49 +1000 schrieb Herbert Xu:
> Here is a minimal fix.
Yes, it fixes the problem.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [BUG 2.6.12-rc4] IPv6 xfrm tunnel oops with large packets
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2005-05-21 20:18 UTC (permalink / raw)
To: herbert; +Cc: christophe, linux-net, netdev, davem
In-Reply-To: <20050521194932.GA10959@gondor.apana.org.au>
In article <20050521194932.GA10959@gondor.apana.org.au> (at Sun, 22 May 2005 05:49:32 +1000), Herbert Xu <herbert@gondor.apana.org.au> says:
> On Sat, May 21, 2005 at 07:10:11PM +0000, Christophe Saout wrote:
> >
> > The oops occurs in line 391 in net/ipv6/icmp.c:
> >
> > > idev = in6_dev_get(skb->dev);
>
> Here is a minimal fix.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> BTW, icmpv6_send seems to ignore its dev argument altogether.
> Any reason why we can't just use it instead of skb->dev?
(After looking into icmpv6_send() usages,)
I don't know the reason why we needed the last argument "dev".
It seems that we already had it at the time of 2.2.16.
Probably, we can remove it.
--yoshfuji
^ permalink raw reply
* Network driver behaviour when loosing link
From: Michael Renzmann @ 2005-05-21 20:05 UTC (permalink / raw)
To: netdev
Hi all.
I'm playing with a network driver and came across one thing I'd like to
ask something about.
Let's assume a driver detects a link status change (for example, the
link is dropped). I would have expected that the driver should signal
the change via netif_carrier_on/_off (if used at all) and tell the
kernel to stop/resume sending packets to the driver (via
netif_wake_queue/netif_stop_queue).
When checking several drivers I found some that call
netif_[wake|stop]_queue (e100, e1000), while others don't (natsemi, tg3).
Hence the question: what is the recommended behaviour in such a case? Is
it ok to call netif_[stop|wake]_queue? How about adjusting the
IFF_RUNNING flag accordingly? Is there any documentation available on
this topic?
Thanks in advance.
Bye, Mike
^ permalink raw reply
* Re: [BUG 2.6.12-rc4] IPv6 xfrm tunnel oops with large packets
From: Herbert Xu @ 2005-05-21 19:49 UTC (permalink / raw)
To: Christophe Saout; +Cc: linux-net, netdev, YOSHIFUJI Hideaki, David S. Miller
In-Reply-To: <1116702611.14509.10.camel@leto.cs.pocnet.net>
[-- Attachment #1: Type: text/plain, Size: 624 bytes --]
On Sat, May 21, 2005 at 07:10:11PM +0000, Christophe Saout wrote:
>
> The oops occurs in line 391 in net/ipv6/icmp.c:
>
> > idev = in6_dev_get(skb->dev);
Here is a minimal fix.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
I wonder why I've never seen it before.
BTW, icmpv6_send seems to ignore its dev argument altogether.
Any reason why we can't just use it instead of skb->dev?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[-- Attachment #2: p --]
[-- Type: text/plain, Size: 274 bytes --]
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -84,6 +84,7 @@ static int xfrm6_tunnel_check_size(struc
mtu = IPV6_MIN_MTU;
if (skb->len > mtu) {
+ skb->dev = dst->dev;
icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu, skb->dev);
ret = -EMSGSIZE;
}
^ permalink raw reply
* [PATCH] netdevice.h: be'ify packet_type
From: Alexey Dobriyan @ 2005-05-21 15:40 UTC (permalink / raw)
To: netdev
Everybody does
struct packet_type foo_packet_type = {
.type = __constant_htons(ETH_P_FOO);
};
5 introduced warnings will be properly fixed later.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
--- linux-20050521140543-000/include/linux/netdevice.h 2005-05-21 14:12:24.000000000 +0400
+++ linux-20050521140543-001/include/linux/netdevice.h 2005-05-21 17:14:26.000000000 +0400
@@ -503,7 +503,7 @@ static inline void *netdev_priv(struct n
#define SET_NETDEV_DEV(net, pdev) ((net)->class_dev.dev = (pdev))
struct packet_type {
- unsigned short type; /* This is really htons(ether_type). */
+ __be16 type; /* This is really htons(ether_type). */
struct net_device *dev; /* NULL is wildcarded here */
int (*func) (struct sk_buff *, struct net_device *,
struct packet_type *);
^ permalink raw reply
* Re: [Netem] [PATCH] (3/3) netem: allow random reordering
From: Julio Kriger @ 2005-05-21 15:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, netdev, netem
In-Reply-To: <20050519151254.79afe7e7@dxpl.pdx.osdl.net>
[-- Attachment #1: Type: text/plain, Size: 6316 bytes --]
Hi!
Here is a patch to 'tc' that add funcionality need to use the new
feature reorder.
Regards,
Julio
On 5/19/05, Stephen Hemminger <shemminger@osdl.org> wrote:
> Enhance the reorder feature of netem to allow random percent to be reordered.
> Has expected backwards compatibility behaviour.
>
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
>
> Index: netem-2.6.12-rc4/include/linux/pkt_sched.h
> ===================================================================
> --- netem-2.6.12-rc4.orig/include/linux/pkt_sched.h
> +++ netem-2.6.12-rc4/include/linux/pkt_sched.h
> @@ -427,6 +427,7 @@ enum
> TCA_NETEM_UNSPEC,
> TCA_NETEM_CORR,
> TCA_NETEM_DELAY_DIST,
> + TCA_NETEM_REORDER,
> __TCA_NETEM_MAX,
> };
>
> @@ -437,7 +438,7 @@ struct tc_netem_qopt
> __u32 latency; /* added delay (us) */
> __u32 limit; /* fifo limit (packets) */
> __u32 loss; /* random packet loss (0=none ~0=100%) */
> - __u32 gap; /* re-ordering gap (0 for delay all) */
> + __u32 gap; /* re-ordering gap (0 for none) */
> __u32 duplicate; /* random packet dup (0=none ~0=100%) */
> __u32 jitter; /* random jitter in latency (us) */
> };
> @@ -449,6 +450,12 @@ struct tc_netem_corr
> __u32 dup_corr; /* duplicate correlation */
> };
>
> +struct tc_netem_reorder
> +{
> + __u32 probability;
> + __u32 correlation;
> +};
> +
> #define NETEM_DIST_SCALE 8192
>
> #endif
> Index: netem-2.6.12-rc4/net/sched/sch_netem.c
> ===================================================================
> --- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
> +++ netem-2.6.12-rc4/net/sched/sch_netem.c
> @@ -62,11 +62,12 @@ struct netem_sched_data {
> u32 gap;
> u32 jitter;
> u32 duplicate;
> + u32 reorder;
>
> struct crndstate {
> unsigned long last;
> unsigned long rho;
> - } delay_cor, loss_cor, dup_cor;
> + } delay_cor, loss_cor, dup_cor, reorder_cor;
>
> struct disttable {
> u32 size;
> @@ -185,18 +186,18 @@ static int netem_enqueue(struct sk_buff
> * of the queue.
> * gap == 0 is special case for no-reordering.
> */
> - if (q->gap == 0 || q->counter != q->gap) {
> + if (q->gap == 0 && q->counter < q->gap &&
> + q->reorder < get_crandom(&q->reorder_cor)) {
> psched_time_t now;
> PSCHED_GET_TIME(now);
> - PSCHED_TADD2(now,
> - tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
> + PSCHED_TADD2(now, tabledist(q->latency, q->jitter,
> + &q->delay_cor, q->delay_dist),
> cb->time_to_send);
> -
> ++q->counter;
> ret = q->qdisc->enqueue(skb, q->qdisc);
> } else {
> - q->counter = 0;
> PSCHED_GET_TIME(cb->time_to_send);
> + q->counter = 0;
> ret = q->qdisc->ops->requeue(skb, q->qdisc);
> }
>
> @@ -351,6 +352,19 @@ static int get_correlation(struct Qdisc
> return 0;
> }
>
> +static int get_reorder(struct Qdisc *sch, const struct rtattr *attr)
> +{
> + struct netem_sched_data *q = qdisc_priv(sch);
> + const struct tc_netem_reorder *r = RTA_DATA(attr);
> +
> + if (RTA_PAYLOAD(attr) != sizeof(*r))
> + return -EINVAL;
> +
> + q->reorder = r->probability;
> + init_crandom(&q->reorder_cor, r->correlation);
> + return 0;
> +}
> +
> static int netem_change(struct Qdisc *sch, struct rtattr *opt)
> {
> struct netem_sched_data *q = qdisc_priv(sch);
> @@ -371,9 +385,15 @@ static int netem_change(struct Qdisc *sc
> q->jitter = qopt->jitter;
> q->limit = qopt->limit;
> q->gap = qopt->gap;
> + q->counter = 0;
> q->loss = qopt->loss;
> q->duplicate = qopt->duplicate;
>
> + /* for compatiablity with earlier versions.
> + * if gap is set, need to assume 100% probablity
> + */
> + q->reorder = ~0;
> +
> /* Handle nested options after initial queue options.
> * Should have put all options in nested format but too late now.
> */
> @@ -395,6 +415,11 @@ static int netem_change(struct Qdisc *sc
> if (ret)
> return ret;
> }
> + if (tb[TCA_NETEM_REORDER-1]) {
> + ret = get_reorder(sch, tb[TCA_NETEM_REORDER-1]);
> + if (ret)
> + return ret;
> + }
> }
>
>
> @@ -412,7 +437,6 @@ static int netem_init(struct Qdisc *sch,
> init_timer(&q->timer);
> q->timer.function = netem_watchdog;
> q->timer.data = (unsigned long) sch;
> - q->counter = 0;
>
> q->qdisc = qdisc_create_dflt(sch->dev, &pfifo_qdisc_ops);
> if (!q->qdisc) {
> @@ -444,6 +468,7 @@ static int netem_dump(struct Qdisc *sch,
> struct rtattr *rta = (struct rtattr *) b;
> struct tc_netem_qopt qopt;
> struct tc_netem_corr cor;
> + struct tc_netem_reorder reorder;
>
> qopt.latency = q->latency;
> qopt.jitter = q->jitter;
> @@ -457,6 +482,11 @@ static int netem_dump(struct Qdisc *sch,
> cor.loss_corr = q->loss_cor.rho;
> cor.dup_corr = q->dup_cor.rho;
> RTA_PUT(skb, TCA_NETEM_CORR, sizeof(cor), &cor);
> +
> + reorder.probability = q->reorder;
> + reorder.correlation = q->reorder_cor.rho;
> + RTA_PUT(skb, TCA_NETEM_REORDER, sizeof(reorder), &reorder);
> +
> rta->rta_len = skb->tail - b;
>
> return skb->len;
>
>
> _______________________________________________
> Netem mailing list
> Netem@lists.osdl.org
> http://lists.osdl.org/mailman/listinfo/netem
>
>
>
--
----------------------------
Julio Kriger
mailto:juliokriger@gmail.com
[-- Attachment #2: tc_patch --]
[-- Type: application/octet-stream, Size: 3661 bytes --]
diff -Naur orig/iproute2-2.6.11-050330/include/linux/pkt_sched.h new/iproute2-2.6.11-050330/include/linux/pkt_sched.h
--- orig/iproute2-2.6.11-050330/include/linux/pkt_sched.h 2005-04-01 19:58:11.000000000 +0000
+++ new/iproute2-2.6.11-050330/include/linux/pkt_sched.h 2005-05-21 11:59:12.000000000 +0000
@@ -427,6 +427,7 @@
TCA_NETEM_UNSPEC,
TCA_NETEM_CORR,
TCA_NETEM_DELAY_DIST,
+ TCA_NETEM_REORDER,
__TCA_NETEM_MAX,
};
@@ -437,7 +438,7 @@
__u32 latency; /* added delay (us) */
__u32 limit; /* fifo limit (packets) */
__u32 loss; /* random packet loss (0=none ~0=100%) */
- __u32 gap; /* re-ordering gap (0 for delay all) */
+ __u32 gap; /* re-ordering gap (0 for none) */
__u32 duplicate; /* random packet dup (0=none ~0=100%) */
__u32 jitter; /* random jitter in latency (us) */
};
@@ -449,6 +450,12 @@
__u32 dup_corr; /* duplicate correlation */
};
+struct tc_netem_reorder
+{
+ __u32 probability;
+ __u32 correlation;
+};
+
#define NETEM_DIST_SCALE 8192
#endif
diff -Naur orig/iproute2-2.6.11-050330/tc/q_netem.c new/iproute2-2.6.11-050330/tc/q_netem.c
--- orig/iproute2-2.6.11-050330/tc/q_netem.c 2005-04-01 19:58:11.000000000 +0000
+++ new/iproute2-2.6.11-050330/tc/q_netem.c 2005-05-21 10:58:33.000000000 +0000
@@ -33,6 +33,7 @@
" [ drop PERCENT [CORRELATION]] \n" \
" [ duplicate PERCENT [CORRELATION]]\n" \
" [ distribution {uniform|normal|pareto|paretonormal} ]\n" \
+" [ reorder PERCENT [CORRELATION]]\n" \
" [ gap PACKETS ]\n");
}
@@ -127,11 +128,13 @@
struct rtattr *tail;
struct tc_netem_qopt opt;
struct tc_netem_corr cor;
+ struct tc_netem_reorder reorder;
__s16 dist_data[MAXDIST];
memset(&opt, 0, sizeof(opt));
opt.limit = 1000;
memset(&cor, 0, sizeof(cor));
+ memset(&reorder, 0, sizeof(reorder));
while (argc > 0) {
if (matches(*argv, "limit") == 0) {
@@ -197,6 +200,19 @@
return -1;
}
}
+ } else if (matches(*argv, "reorder") == 0) {
+ NEXT_ARG();
+ if (get_percent(&reorder.probability, *argv)) {
+ explain1("reorder");
+ return -1;
+ }
+ if (NEXT_IS_NUMBER()) {
+ NEXT_ARG();
+ if (get_percent(&reorder.correlation, *argv)) {
+ explain1("reorder");
+ return -1;
+ }
+ }
} else if (matches(*argv, "distribution") == 0) {
NEXT_ARG();
dist_size = get_distribution(*argv, dist_data);
@@ -217,6 +233,7 @@
addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
addattr_l(n, 1024, TCA_NETEM_CORR, &cor, sizeof(cor));
+ addattr_l(n, 1024, TCA_NETEM_REORDER, &reorder, sizeof(reorder));
if (dist_size > 0) {
addattr_l(n, 32768, TCA_NETEM_DELAY_DIST,
@@ -229,7 +246,8 @@
static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
{
const struct tc_netem_corr *cor = NULL;
- struct tc_netem_qopt qopt;
+ const struct tc_netem_reorder *reorder = NULL;
+ struct tc_netem_qopt qopt;
int len = RTA_PAYLOAD(opt) - sizeof(qopt);
SPRINT_BUF(b1);
@@ -252,6 +270,12 @@
return -1;
cor = RTA_DATA(tb[TCA_NETEM_CORR]);
}
+
+ if (tb[TCA_NETEM_REORDER]) {
+ if (RTA_PAYLOAD(tb[TCA_NETEM_REORDER]) < sizeof(*reorder))
+ return -1;
+ reorder = RTA_DATA(tb[TCA_NETEM_REORDER]);
+ }
}
fprintf(f, "limit %d", qopt.limit);
@@ -279,6 +303,13 @@
fprintf(f, " %s", sprint_percent(cor->dup_corr, b1));
}
+ if (reorder && reorder->probability) {
+ fprintf(f, " reorder %s",
+ sprint_percent(reorder->probability, b1));
+ if (reorder && reorder->correlation)
+ fprintf(f, " %s", sprint_percent(reorder->correlation, b1));
+ }
+
if (qopt.gap)
fprintf(f, " gap %lu", (unsigned long)qopt.gap);
^ permalink raw reply
* who is eating hw header in packets?
From: cranium2003 @ 2005-05-21 9:48 UTC (permalink / raw)
To: net dev
hello,
I use ethereal to capture a packet. I
sent a ping request packet to hosta from hostB. On
hostA it will be captured and replyis generated and
packet sent to hostB. But it is captured as
Frame 2 (98 bytes on wire, 98 bytes captured)
Arrival Time: May 21, 2005 10:37:57.360894000
Time delta from previous packet: 0.000409000
seconds
Time relative to first packet: 0.000409000 seconds
Frame Number: 2
Packet Length: 98 bytes
Capture Length: 98 bytes
Ethernet II, Src: 00:00:40:01:bc:29, Dst:
45:00:00:54:3a:2b
Destination: 45:00:00:54:3a:2b (45:00:00:54:3a:2b)
Source: 00:00:40:01:bc:29 (Applicon_01:bc:29)
Type: Unknown (0xc0a8)
Data (84 bytes)
0000 01 fa c0 a8 01 0a 00 00 82 68 0e 0f 00 01 2d c2
.........h....-.
0010 8e 42 c3 7f 05 00 08 09 0a 0b 0c 0d 0e 0f 10 11
.B..............
0020 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21
.............. !
0030 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31
"#$%&'()*+,-./01
0040 32 33 34 35 36 37 00 00 00 00 00 00 00 00 00 00
234567..........
0050 00 00 00 00
....
I want to ask which routine in linux kernel
is discarding packet? Because what i found is that the
first 14 bytes((src + dst) HW header + protocol type
field)
00 08 a1 43 61 f5 00 08 a1 43 62 91 08 00 bytes are
replaced with next all bytes and last 14 bytes are
filled with 0's.
regards,
cranium.
__________________________________
Yahoo! Mail Mobile
Take Yahoo! Mail with you! Check email on your mobile phone.
http://mobile.yahoo.com/learn/mail
^ permalink raw reply
* Re: A new driver for Broadcom bcm5706
From: David S. Miller @ 2005-05-21 4:36 UTC (permalink / raw)
To: mchan; +Cc: jgarzik, netdev, ffan, lusinsky
In-Reply-To: <1116630261.31523.42.camel@rh4>
From: "Michael Chan" <mchan@broadcom.com>
Date: Fri, 20 May 2005 16:04:21 -0700
> On Fri, 2005-05-20 at 15:28 -0700, David S.Miller wrote:
> > From: Jeff Garzik <jgarzik@pobox.com>
> > Date: Fri, 20 May 2005 15:42:20 -0400
> >
> > > 9) [additional review] DaveM, others: is this correct for all arches?
> > >
> > > + if (unlikely((align = (unsigned long) skb->data & 0x7))) {
> > > + skb_reserve(skb, 8 - align);
> > > + }
> >
> > It's probably not even necessary. dev_alloc_skb() should be returning
> > an SKB with skb->data at least cache_line_size() aligned (see mm/slab.c)
> > unless the platform defines an ARCH_KMALLOC_MINALIGN override.
>
> If I remember correctly, I was seeing some SKB with skb->data that is 4-
> byte aligned on some Fedora kernels. I don't remember which kernel. This
> device has an alignment requirement of at least 8-bytes for the receive
> buffers.
Just keep it in for now then. I wouldn't be surprised if some 2.4.x
kernels let this happen.
> Yes, but in this case, cksum is the checksum calculated over the entire
> TCP/UDP packet including the pseudo IP header. Jeff is right, it should
> always be 0xffff when the checksum is correct. Checking for zero is a
> bug.
Ok, thanks for verifying.
> Originally, the driver pulse interval was set at 250 msec, but it's been
> extended to a few seconds. So the driver currently will write the pulse
> every second and do the serdes related checking at the same time.
I think the current timer stuff is fine, at least for now.
^ permalink raw reply
* Re: A new driver for Broadcom bcm5706
From: David S. Miller @ 2005-05-21 4:35 UTC (permalink / raw)
To: mchan; +Cc: jgarzik, netdev, ffan, lusinsky
In-Reply-To: <1116630261.31523.42.camel@rh4>
From: "Michael Chan" <mchan@broadcom.com>
Date: Fri, 20 May 2005 16:04:21 -0700
> On Fri, 2005-05-20 at 15:28 -0700, David S.Miller wrote:
> David, Do you want me to fix some of these things and send you a new
> 500K patch or just send incremental patches over the existing driver?
Please send incremental patches. That also makes the
changes easier to review as well.
^ permalink raw reply
* Re: A new driver for Broadcom bcm5706
From: David S. Miller @ 2005-05-21 4:28 UTC (permalink / raw)
To: jgarzik; +Cc: mchan, netdev, ffan, lusinsky
In-Reply-To: <428E7A53.1030907@pobox.com>
From: Jeff Garzik <jgarzik@pobox.com>
Subject: Re: A new driver for Broadcom bcm5706
Date: Fri, 20 May 2005 20:01:23 -0400
> David S.Miller wrote:
> > From: Jeff Garzik <jgarzik@pobox.com>
> > Date: Fri, 20 May 2005 19:30:01 -0400
> >
> >
> >>Sure. What I'm driving at is that a checksum of zero seems to imply
> >>CHECKSUM_NONE not CHECKSUM_UNNECESSARY. tg3 only does the 0xffff check.
> >
> >
> > Sure, both ways are fine.
>
> huh? They are pretty different... one says "Checksum all good, dude"
> and the other says "I didn't checksum, do it in software for me."
>
> right?
0x0000 is the UDP "no checksum" case, so if we say "in software"
for that UDP will just let it pass through still, so the effect
is that same.
^ 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