* Re: [PATCH net-next 0/4] ipv6: fix the reassembly expire code in nf_conntrack
From: Cong Wang @ 2012-09-17 3:13 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, Herbert Xu, David S. Miller
In-Reply-To: <1347517541-10653-1-git-send-email-amwang@redhat.com>
Ping... Any review? :)
On Thu, 2012-09-13 at 14:25 +0800, Cong Wang wrote:
> ipv6: add a new namespace for nf_conntrack_reasm
> ipv6: unify conntrack reassembly expire code with
> ipv6: make ip6_frag_nqueues() and ip6_frag_mem() static
> ipv6: unify fragment thresh handling code
>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>
>
> ---
>
> include/net/inet_frag.h | 2 +-
> include/net/ipv6.h | 32 +++++-
> include/net/net_namespace.h | 3 +
> include/net/netns/conntrack.h | 6 +
> net/ipv4/inet_fragment.c | 9 +-
> net/ipv4/ip_fragment.c | 5 +-
> net/ipv6/netfilter/nf_conntrack_reasm.c | 196 ++++++++++++++++---------------
> net/ipv6/reassembly.c | 88 ++++----------
> 8 files changed, 176 insertions(+), 165 deletions(-)
>
^ permalink raw reply
* [TCP]: functionality of delayed_ack in Bic and Cubic Algorithm ?
From: Yi Li @ 2012-09-17 2:34 UTC (permalink / raw)
To: netdev
In-Reply-To: <5056897A.9010009@gmail.com>
Hi All,
I am try to understand the patch:
http://patchwork.usersys.redhat.com/patch/43827/.
But I am not sure of the functionality of delayed_ack filed in Bic and
Cubic.
I have found the following mails:
http://oss.sgi.com/archives/netdev/2005-02/msg00808.html
which is the first patch introducing the *delayed_ack* field.
( But I am not fully understand that material, That's why I have asked
here.)
So, here is my understanding of this field, and I am not sure whether it
is right. :-(
Question One:
>From comment in *struct bictcp*, delayed_ack is "the ratio of
Packets/ACKs << 4"
and it's updating is in function bictcp_acked():
/* Track delayed acknowledgment ratio using sliding window
* ratio = (15*ratio + sample) / 16
*/
static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt_us)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
const struct tcp_sock *tp = tcp_sk(sk);
struct bictcp *ca = inet_csk_ca(sk);
u32 delay;
if (icsk->icsk_ca_state == TCP_CA_Open) {
cnt -= ca->delayed_ack >> ACK_RATIO_SHIFT;
ca->delayed_ack += cnt;
}
After googling, I know ratio == delayed_ack >> ACK_RATIO_SHIFT. so here
we are updating
the Packets/Acks ratio, basing on the history of ratio (15/16) and the
current ratio(1/16).
The current ratio is cnt packets acked by the current acknowledgement,
divided by the current
count of acknowledgements(of course it is 1 ack packet). Right?
Question Two:
And we update the ca->cnt in function bictcp_update():
ca->cnt = (ca->cnt << ACK_RATIO_SHIFT) / ca->delayed_ack;
if (ca->cnt == 0) /* cannot be zero */
ca->cnt = 1;
It means ca->cnt= ca->cnt * Acks/Packets. Suppose normal delayed ack,
Acks/Packets should be 1/2.
So, ca->cnt will be cut in half. As a result, snd_cwnd will increase one
times more rapidly, and this is just a
compensation for delayed ack. So, TCP will still work normally. Right?
^ permalink raw reply
* Goodluck
From: Allen and Violet Large @ 2012-09-17 0:30 UTC (permalink / raw)
This is a personal email directed to you. My wife and I won a Jackpot
Lottery of $11.3 million in July and have voluntarily decided to donate
the sum of $500,000.00 USD to you as part of our own charity project to
improve the lot of 10 lucky individuals all over the world. If you have
received this email then you are one of the lucky recipients and all you
have to do is get back with us so that we can send your details to the
payout bank.
Please note that you have to contact my private email for more
informations (allen_violetlarge03@yahoo.co.jp)
You can verify this by visiting the web pages below.
http://www.dailymail.co.uk/news/article-1326473/Canadian-couple-Allen-Violet-Large-away-entire-11-2m-lottery-win.html
http://www.cbc.ca/news/canada/nova-scotia/story/2010/11/04/ns-allen-violet-large-lottery-winning.html
Goodluck,
Allen and Violet Large
Email: allen_violetlarge03@yahoo.co.jp
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: provide a default dev->ethtool_ops
From: Ben Hutchings @ 2012-09-16 22:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Maciej Żenczykowski
In-Reply-To: <1347823046.26523.44.camel@edumazet-glaptop>
On Sun, 2012-09-16 at 21:17 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> On Sat, 2012-09-15 at 14:40 +0100, Ben Hutchings wrote:
>
> > > strcpy(dev->name, name);
> > > dev->group = INIT_NETDEV_GROUP;
> > > + if (!dev->ethtool_ops) {
> > > + static const struct ethtool_ops default_ethtool_ops;
> > > +
> > > + dev->ethtool_ops = &default_ethtool_ops;
> > > + }
> >
> > This block has a blank line in it, so I think it needs a blank line
> > either side to make the visual grouping of code right. Alternately you
> > could pull the variable out of the block.
> >
>
> Blank line is preferred after a variable declaration in a function.
Yes but:
when the previous statement is right next to;
the start of the block {
the variable declaration is visually grouped with that;
rather than with the statement that uses it;
}
> I dont feel the need to make this variable visible outside of this
> function yet. But if you feel it, no problem.
>
> > [...]
> > > @@ -1410,8 +1409,9 @@ static int ethtool_get_module_eeprom(struct net_device *dev,
> > > modinfo.eeprom_len);
> > > }
> > >
> > > -/* The main entry point in this file. Called from net/core/dev.c */
> > > -
> > > +/* The main entry point in this file. Called from net/core/dev.c
> > > + * with RTNL held.
> > > + */
> >
> > Good point but an unrelated change.
>
> Its related, because I wanted to make clear (at least for me)
> why assuming dev->ethtool_ops was not NULL at this point was valid.
So we know it won't change under us? But that is also true of
dev->netdev_ops, which is often used with a finer-grained lock.
> > > int dev_ethtool(struct net *net, struct ifreq *ifr)
> > > {
> > > struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
> > > @@ -1419,25 +1419,15 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> > > u32 ethcmd;
> > > int rc;
> > > u32 old_features;
> > > + const struct ethtool_ops *ops;
> > >
> > > if (!dev || !netif_device_present(dev))
> > > return -ENODEV;
> > >
> > > + ops = dev->ethtool_ops;
> > [...]
> >
> > Introducing this local variable is a useful cleanup but again should be
> > a separate change.
>
> Its a patch meant for net-next, and a cleanup. I could understand your
> point if we had to backport this to stable trees, buts its not the
> case ?
>
> I really dont care, so if you really prefer I dont cleanup ethtool.c, so
> be it.
>
> Some functions test dev->ethtool_ops is NULL, others lack this test.
> This just makes no sense to me, maybe there is something I missed.
Most of those checks are probably redundant, and I see no problem with
your removing them. It was just that you were also changing various
other references to dev->ethtool_ops to use local variables.
> Thanks
>
> [PATCH v2 net-next] net: provide a default dev->ethtool_ops
>
> Instead of forcing device drivers to provide empty ethtool_ops or tweak
> net/core/ethtool.c again, we could provide a generic ethtool_ops.
>
> This occurred to me when I wanted to add GSO support to GRE tunnels.
> ethtool -k support should be generic for all drivers.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Ben Hutchings <bhutchings@solarflare.com>
> Cc: Maciej Żenczykowski <maze@google.com>
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> net/core/dev.c | 4 ++++
> net/core/ethtool.c | 12 ------------
> 2 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index dcc673d..2bcb02c 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5959,6 +5959,8 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
> return queue;
> }
>
> +static const struct ethtool_ops default_ethtool_ops;
> +
> /**
> * alloc_netdev_mqs - allocate network device
> * @sizeof_priv: size of private data to allocate space for
> @@ -6046,6 +6048,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>
> strcpy(dev->name, name);
> dev->group = INIT_NETDEV_GROUP;
> + if (!dev->ethtool_ops)
> + dev->ethtool_ops = &default_ethtool_ops;
> return dev;
>
> free_all:
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index cbf033d..4d64cc2 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -1426,18 +1426,6 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> if (copy_from_user(ðcmd, useraddr, sizeof(ethcmd)))
> return -EFAULT;
>
> - if (!dev->ethtool_ops) {
> - /* A few commands do not require any driver support,
> - * are unprivileged, and do not change anything, so we
> - * can take a shortcut to them. */
> - if (ethcmd == ETHTOOL_GDRVINFO)
> - return ethtool_get_drvinfo(dev, useraddr);
> - else if (ethcmd == ETHTOOL_GET_TS_INFO)
> - return ethtool_get_ts_info(dev, useraddr);
> - else
> - return -EOPNOTSUPP;
> - }
> -
> /* Allow some commands to be done by anyone */
> switch (ethcmd) {
> case ETHTOOL_GSET:
>
>
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: provide a default dev->ethtool_ops
From: Eric Dumazet @ 2012-09-16 19:17 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, netdev, Maciej Żenczykowski
In-Reply-To: <1347716447.13258.101.camel@deadeye.wl.decadent.org.uk>
From: Eric Dumazet <edumazet@google.com>
On Sat, 2012-09-15 at 14:40 +0100, Ben Hutchings wrote:
> > strcpy(dev->name, name);
> > dev->group = INIT_NETDEV_GROUP;
> > + if (!dev->ethtool_ops) {
> > + static const struct ethtool_ops default_ethtool_ops;
> > +
> > + dev->ethtool_ops = &default_ethtool_ops;
> > + }
>
> This block has a blank line in it, so I think it needs a blank line
> either side to make the visual grouping of code right. Alternately you
> could pull the variable out of the block.
>
Blank line is preferred after a variable declaration in a function.
I dont feel the need to make this variable visible outside of this
function yet. But if you feel it, no problem.
> [...]
> > @@ -1410,8 +1409,9 @@ static int ethtool_get_module_eeprom(struct net_device *dev,
> > modinfo.eeprom_len);
> > }
> >
> > -/* The main entry point in this file. Called from net/core/dev.c */
> > -
> > +/* The main entry point in this file. Called from net/core/dev.c
> > + * with RTNL held.
> > + */
>
> Good point but an unrelated change.
Its related, because I wanted to make clear (at least for me)
why assuming dev->ethtool_ops was not NULL at this point was valid.
>
> > int dev_ethtool(struct net *net, struct ifreq *ifr)
> > {
> > struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
> > @@ -1419,25 +1419,15 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
> > u32 ethcmd;
> > int rc;
> > u32 old_features;
> > + const struct ethtool_ops *ops;
> >
> > if (!dev || !netif_device_present(dev))
> > return -ENODEV;
> >
> > + ops = dev->ethtool_ops;
> [...]
>
> Introducing this local variable is a useful cleanup but again should be
> a separate change.
Its a patch meant for net-next, and a cleanup. I could understand your
point if we had to backport this to stable trees, buts its not the
case ?
I really dont care, so if you really prefer I dont cleanup ethtool.c, so
be it.
Some functions test dev->ethtool_ops is NULL, others lack this test.
This just makes no sense to me, maybe there is something I missed.
Thanks
[PATCH v2 net-next] net: provide a default dev->ethtool_ops
Instead of forcing device drivers to provide empty ethtool_ops or tweak
net/core/ethtool.c again, we could provide a generic ethtool_ops.
This occurred to me when I wanted to add GSO support to GRE tunnels.
ethtool -k support should be generic for all drivers.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Ben Hutchings <bhutchings@solarflare.com>
Cc: Maciej Żenczykowski <maze@google.com>
---
net/core/dev.c | 4 ++++
net/core/ethtool.c | 12 ------------
2 files changed, 4 insertions(+), 12 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index dcc673d..2bcb02c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5959,6 +5959,8 @@ struct netdev_queue *dev_ingress_queue_create(struct net_device *dev)
return queue;
}
+static const struct ethtool_ops default_ethtool_ops;
+
/**
* alloc_netdev_mqs - allocate network device
* @sizeof_priv: size of private data to allocate space for
@@ -6046,6 +6048,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
strcpy(dev->name, name);
dev->group = INIT_NETDEV_GROUP;
+ if (!dev->ethtool_ops)
+ dev->ethtool_ops = &default_ethtool_ops;
return dev;
free_all:
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index cbf033d..4d64cc2 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1426,18 +1426,6 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
if (copy_from_user(ðcmd, useraddr, sizeof(ethcmd)))
return -EFAULT;
- if (!dev->ethtool_ops) {
- /* A few commands do not require any driver support,
- * are unprivileged, and do not change anything, so we
- * can take a shortcut to them. */
- if (ethcmd == ETHTOOL_GDRVINFO)
- return ethtool_get_drvinfo(dev, useraddr);
- else if (ethcmd == ETHTOOL_GET_TS_INFO)
- return ethtool_get_ts_info(dev, useraddr);
- else
- return -EOPNOTSUPP;
- }
-
/* Allow some commands to be done by anyone */
switch (ethcmd) {
case ETHTOOL_GSET:
^ permalink raw reply related
* Hello
From: Linda Sajid @ 2012-09-16 17:05 UTC (permalink / raw)
Hello,
My names is Linda. How are you doing?
Hope fine. I wish to request for your true
friendship.
^ permalink raw reply
* Re: skb_linearize
From: Ben Hutchings @ 2012-09-16 15:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, Herbert Xu
In-Reply-To: <20120916091747.GA23775@redhat.com>
On Sun, 2012-09-16 at 12:17 +0300, Michael S. Tsirkin wrote:
> I notice that dev_hard_start_xmit might invoke
> __skb_linearize e.g. if device does not support NETIF_F_SG.
>
> This in turn onvokes __pskb_pull_tail, and
> documentation of __pskb_pull_tail says:
> &sk_buff MUST have reference count of 1.
>
> I am guessing 'reference count' means users in this context, right?
> IIUC this is because it modifies skb in a way that
> isn't safe if anyone else is looking at the skb.
>
>
> However, I don't see what guarantees that reference
> count is 1 when dev_hard_start_xmit invokes
> linearize. In particular it calls dev_queue_xmit_nit
> which could queue packets on a network tap.
>
> Could someone help me understand please?
Reference count here means references to struct sk_buff itself. The
header area and data fragments are allowed to be shared.
dev_queue_xmit_nit() clones the skb for each tap, so the reference count
on the original skb remains 1.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* skb_linearize
From: Michael S. Tsirkin @ 2012-09-16 9:17 UTC (permalink / raw)
To: netdev, Herbert Xu
I notice that dev_hard_start_xmit might invoke
__skb_linearize e.g. if device does not support NETIF_F_SG.
This in turn onvokes __pskb_pull_tail, and
documentation of __pskb_pull_tail says:
&sk_buff MUST have reference count of 1.
I am guessing 'reference count' means users in this context, right?
IIUC this is because it modifies skb in a way that
isn't safe if anyone else is looking at the skb.
However, I don't see what guarantees that reference
count is 1 when dev_hard_start_xmit invokes
linearize. In particular it calls dev_queue_xmit_nit
which could queue packets on a network tap.
Could someone help me understand please?
Thanks!
--
MST
^ permalink raw reply
* [PATCH] net: fix memory leak on oom with zerocopy
From: Michael S. Tsirkin @ 2012-09-16 8:44 UTC (permalink / raw)
Cc: David S. Miller, Eric Dumazet, Ben Hutchings,
Michał Mirosław, netdev, linux-kernel
If orphan flags fails, we don't free the skb
on receive, which leaks the skb memory.
Return value was also wrong: netif_receive_skb
is supposed to return NET_RX_DROP, not ENOMEM.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Fixes a memory leak so 3.6 material?
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 8398836..899f827 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3321,7 +3321,7 @@ ncls:
if (pt_prev) {
if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
- ret = -ENOMEM;
+ goto drop;
else
ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
} else {
--
MST
^ permalink raw reply related
* Re: bnx2 cards intermittantly going offline
From: Ben Hutchings @ 2012-09-16 3:47 UTC (permalink / raw)
To: Sven Ulland; +Cc: netdev, Marc A. Donges, Michael Chan
In-Reply-To: <5051FFAA.8060501@opera.com>
[-- Attachment #1: Type: text/plain, Size: 2520 bytes --]
On Thu, 2012-09-13 at 17:45 +0200, Sven Ulland wrote:
> On 09/13/2012 03:51 PM, Marc A. Donges wrote:
> > After 55 days of operation the machine (A) suddenly was no longer
> > reachable via network. Strangely, a second machine (B) that should
> > take over the IP addresses (keepalived) did not take over. Only
> > after shutting the switchport to which A is attached did B take
> > over.
>
> Hi. We've had the same symptom with our BCM5709S [14e4:163a] on
> Debian. Like you, we were on stable's 2.6.32-41squeeze2. Google led us
> to many similar issues [1,2,3]. They concluded with the fix being in
> mainline commit c441b8d2 [4]: "bnx2: Fix lost MSI-X problem on 5709
> NICs".
>
> Broadcom: Can you publish a tool that decodes ethtool -d dumps to make
> debugging easier, or do you deem it no longer necessary with the the
> register dump commits in 555069da?
This tool should be ethtool itself (it includes dump decoders for many
drivers).
> Now, Debian's 2.6.32-41squeeze2 is based on longterm release 2.6.32.54
> [5]. That version includes commit 0b7817ed [6], which is a backport of
> the already mentioned mainline commit c441b8d2.
>
> So we tried digging further and applying some seemingly relevant
> commits [7,8] to our 2.6.32, but without any change in behaviour. Our
> temporary fix was to run 'ethtool -t ethX' to reset the device every
> time it locked up.
>
> This dragged on with various builds, until we ended up on mainline
> 2.6.38 where we no longer saw any symptoms. I don't know in which
> kernel version it was fixed, but we ended up on that one, sort of by
> chance. Unfortunately, it had severe issues with kswapd memory
> compaction causing CPU soft lockups [9], so we went straight to
> squeeze-backports' 3.2.23-1~bpo60+2. We've been happy since then.
>
> > We have five pairs of basically identical machines performing the
> > same task (each pair for one site). The error has not occured with
> > any other one, but this site is the busiest:
>
> We also saw the issue only at a site with generally higher load
> compared to other sites.
>
> I'd love to know exactly which commit fixed the issue, but it's fairly
> tricky to reproduce the issue, and the bisect count is fairly high (it
> need not be a specific fix for bnx2).
I don't see any changes to the driver itself that look relevant.
Perhaps this was a firmware bug?
Ben.
--
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH 0/6] llc2: Simplify llc_station
From: Ben Hutchings @ 2012-09-16 3:13 UTC (permalink / raw)
To: David S. Miller, Arnaldo Carvalho de Melo; +Cc: netdev
In-Reply-To: <1347764982.13258.207.camel@deadeye.wl.decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 397 bytes --]
On Sun, 2012-09-16 at 04:09 +0100, Ben Hutchings wrote:
> There seem to have been some grand plans for llc_station, but as they
> haven't been fulfilled it's just unnecessarily complicated.
Oh, a warning: this is compile-tested only. I don't even know what I
would test this with.
Ben.
--
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* [PATCH net-next 6/6] llc2: Collapse remainder of state machine into simple if-else if-statement
From: Ben Hutchings @ 2012-09-16 3:11 UTC (permalink / raw)
To: David S. Miller, Arnaldo Carvalho de Melo; +Cc: netdev
In-Reply-To: <1347764982.13258.207.camel@deadeye.wl.decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 3669 bytes --]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
net/llc/llc_station.c | 91 +++----------------------------------------------
1 file changed, 4 insertions(+), 87 deletions(-)
diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index fe43158..204a835 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -25,16 +25,6 @@
#include <net/llc_s_st.h>
#include <net/llc_pdu.h>
-typedef int (*llc_station_ev_t)(struct sk_buff *skb);
-
-typedef int (*llc_station_action_t)(struct sk_buff *skb);
-
-/* Station component state table structure */
-struct llc_station_state_trans {
- llc_station_ev_t ev;
- llc_station_action_t *ev_actions;
-};
-
static int llc_stat_ev_rx_null_dsap_xid_c(struct sk_buff *skb)
{
struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
@@ -109,78 +99,6 @@ free:
goto out;
}
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_XID_C event */
-static llc_station_action_t llc_stat_up_state_actions_2[] = {
- llc_station_ac_send_xid_r,
- NULL,
-};
-
-static struct llc_station_state_trans llc_stat_up_state_trans_2 = {
- .ev = llc_stat_ev_rx_null_dsap_xid_c,
- .ev_actions = llc_stat_up_state_actions_2,
-};
-
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_TEST_C event */
-static llc_station_action_t llc_stat_up_state_actions_3[] = {
- llc_station_ac_send_test_r,
- NULL,
-};
-
-static struct llc_station_state_trans llc_stat_up_state_trans_3 = {
- .ev = llc_stat_ev_rx_null_dsap_test_c,
- .ev_actions = llc_stat_up_state_actions_3,
-};
-
-/* array of pointers; one to each transition */
-static struct llc_station_state_trans *llc_stat_up_state_trans [] = {
- &llc_stat_up_state_trans_2,
- &llc_stat_up_state_trans_3,
- NULL,
-};
-
-/**
- * llc_exec_station_trans_actions - executes actions for transition
- * @trans: Address of the transition
- * @skb: Address of the event that caused the transition
- *
- * Executes actions of a transition of the station state machine. Returns
- * 0 if all actions complete successfully, nonzero otherwise.
- */
-static u16 llc_exec_station_trans_actions(struct llc_station_state_trans *trans,
- struct sk_buff *skb)
-{
- u16 rc = 0;
- llc_station_action_t *next_action = trans->ev_actions;
-
- for (; next_action && *next_action; next_action++)
- if ((*next_action)(skb))
- rc = 1;
- return rc;
-}
-
-/**
- * llc_find_station_trans - finds transition for this event
- * @skb: Address of the event
- *
- * Search thru events of the current state of the station until list
- * exhausted or it's obvious that the event is not valid for the current
- * state. Returns the address of the transition if cound, %NULL otherwise.
- */
-static struct llc_station_state_trans *
- llc_find_station_trans(struct sk_buff *skb)
-{
- int i = 0;
- struct llc_station_state_trans *rc = NULL;
- struct llc_station_state_trans **next_trans;
-
- for (next_trans = llc_stat_up_state_trans; next_trans[i]; i++)
- if (!next_trans[i]->ev(skb)) {
- rc = next_trans[i];
- break;
- }
- return rc;
-}
-
/**
* llc_station_rcv - send received pdu to the station state machine
* @skb: received frame.
@@ -189,11 +107,10 @@ static struct llc_station_state_trans *
*/
static void llc_station_rcv(struct sk_buff *skb)
{
- struct llc_station_state_trans *trans;
-
- trans = llc_find_station_trans(skb);
- if (trans)
- llc_exec_station_trans_actions(trans, skb);
+ if (llc_stat_ev_rx_null_dsap_xid_c(skb))
+ llc_station_ac_send_xid_r(skb);
+ else if (llc_stat_ev_rx_null_dsap_test_c(skb))
+ llc_station_ac_send_test_r(skb);
kfree_skb(skb);
}
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* [PATCH net-next 5/6] llc2: Remove explicit indexing of state action arrays
From: Ben Hutchings @ 2012-09-16 3:11 UTC (permalink / raw)
To: David S. Miller, Arnaldo Carvalho de Melo; +Cc: netdev
In-Reply-To: <1347764982.13258.207.camel@deadeye.wl.decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]
These arrays are accessed by iteration in
llc_exec_station_trans_actions(). There must not be any zero-filled
gaps in them, so the explicit indices are pointless.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
net/llc/llc_station.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index 48c2118..fe43158 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -111,8 +111,8 @@ free:
/* state transition for LLC_STATION_EV_RX_NULL_DSAP_XID_C event */
static llc_station_action_t llc_stat_up_state_actions_2[] = {
- [0] = llc_station_ac_send_xid_r,
- [1] = NULL,
+ llc_station_ac_send_xid_r,
+ NULL,
};
static struct llc_station_state_trans llc_stat_up_state_trans_2 = {
@@ -122,8 +122,8 @@ static struct llc_station_state_trans llc_stat_up_state_trans_2 = {
/* state transition for LLC_STATION_EV_RX_NULL_DSAP_TEST_C event */
static llc_station_action_t llc_stat_up_state_actions_3[] = {
- [0] = llc_station_ac_send_test_r,
- [1] = NULL,
+ llc_station_ac_send_test_r,
+ NULL,
};
static struct llc_station_state_trans llc_stat_up_state_trans_3 = {
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* [PATCH net-next 4/6] llc2: Remove the station send queue
From: Ben Hutchings @ 2012-09-16 3:11 UTC (permalink / raw)
To: David S. Miller, Arnaldo Carvalho de Melo; +Cc: netdev
In-Reply-To: <1347764982.13258.207.camel@deadeye.wl.decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2637 bytes --]
We only ever put one skb on the send queue, and then immediately
send it. Remove the queue and call dev_queue_xmit() directly.
This leaves struct llc_station empty, so remove that as well.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
net/llc/llc_station.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index 3bdb888..48c2118 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -25,19 +25,6 @@
#include <net/llc_s_st.h>
#include <net/llc_pdu.h>
-/**
- * struct llc_station - LLC station component
- *
- * SAP and connection resource manager, one per adapter.
- *
- * @mac_sa: MAC source address
- * @sap_list: list of related SAPs
- * @mac_pdu_q: PDUs ready to send to MAC
- */
-struct llc_station {
- struct sk_buff_head mac_pdu_q;
-};
-
typedef int (*llc_station_ev_t)(struct sk_buff *skb);
typedef int (*llc_station_action_t)(struct sk_buff *skb);
@@ -48,8 +35,6 @@ struct llc_station_state_trans {
llc_station_action_t *ev_actions;
};
-static struct llc_station llc_main_station;
-
static int llc_stat_ev_rx_null_dsap_xid_c(struct sk_buff *skb)
{
struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
@@ -70,20 +55,6 @@ static int llc_stat_ev_rx_null_dsap_test_c(struct sk_buff *skb)
!pdu->dsap ? 0 : 1; /* NULL DSAP */
}
-/**
- * llc_station_send_pdu - queues PDU to send
- * @skb: Address of the PDU
- *
- * Queues a PDU to send to the MAC layer.
- */
-static void llc_station_send_pdu(struct sk_buff *skb)
-{
- skb_queue_tail(&llc_main_station.mac_pdu_q, skb);
- while ((skb = skb_dequeue(&llc_main_station.mac_pdu_q)) != NULL)
- if (dev_queue_xmit(skb))
- break;
-}
-
static int llc_station_ac_send_xid_r(struct sk_buff *skb)
{
u8 mac_da[ETH_ALEN], dsap;
@@ -101,7 +72,7 @@ static int llc_station_ac_send_xid_r(struct sk_buff *skb)
rc = llc_mac_hdr_init(nskb, skb->dev->dev_addr, mac_da);
if (unlikely(rc))
goto free;
- llc_station_send_pdu(nskb);
+ dev_queue_xmit(nskb);
out:
return rc;
free:
@@ -130,7 +101,7 @@ static int llc_station_ac_send_test_r(struct sk_buff *skb)
rc = llc_mac_hdr_init(nskb, skb->dev->dev_addr, mac_da);
if (unlikely(rc))
goto free;
- llc_station_send_pdu(nskb);
+ dev_queue_xmit(nskb);
out:
return rc;
free:
@@ -228,7 +199,6 @@ static void llc_station_rcv(struct sk_buff *skb)
void __init llc_station_init(void)
{
- skb_queue_head_init(&llc_main_station.mac_pdu_q);
llc_set_station_handler(llc_station_rcv);
}
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* [PATCH net-next 3/6] llc2: Collapse the station event receive path
From: Ben Hutchings @ 2012-09-16 3:11 UTC (permalink / raw)
To: David S. Miller, Arnaldo Carvalho de Melo; +Cc: netdev
In-Reply-To: <1347764982.13258.207.camel@deadeye.wl.decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 3942 bytes --]
We only ever put one skb on the event queue, and then immediately
process it. Remove the queue and fold together the related functions,
removing several blatantly false comments.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
net/llc/llc_station.c | 87 ++++---------------------------------------------
1 file changed, 6 insertions(+), 81 deletions(-)
diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index 917d700..3bdb888 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -32,14 +32,9 @@
*
* @mac_sa: MAC source address
* @sap_list: list of related SAPs
- * @ev_q: events entering state mach.
* @mac_pdu_q: PDUs ready to send to MAC
*/
struct llc_station {
- struct {
- struct sk_buff_head list;
- spinlock_t lock;
- } ev_q;
struct sk_buff_head mac_pdu_q;
};
@@ -216,79 +211,6 @@ static struct llc_station_state_trans *
}
/**
- * llc_station_free_ev - frees an event
- * @skb: Address of the event
- *
- * Frees an event.
- */
-static void llc_station_free_ev(struct sk_buff *skb)
-{
- kfree_skb(skb);
-}
-
-/**
- * llc_station_next_state - processes event and goes to the next state
- * @skb: Address of the event
- *
- * Processes an event, executes any transitions related to that event and
- * updates the state of the station.
- */
-static u16 llc_station_next_state(struct sk_buff *skb)
-{
- u16 rc = 1;
- struct llc_station_state_trans *trans;
-
- trans = llc_find_station_trans(skb);
- if (trans)
- /* got the state to which we next transition; perform the
- * actions associated with this transition before actually
- * transitioning to the next state
- */
- rc = llc_exec_station_trans_actions(trans, skb);
- else
- /* event not recognized in current state; re-queue it for
- * processing again at a later time; return failure
- */
- rc = 0;
- llc_station_free_ev(skb);
- return rc;
-}
-
-/**
- * llc_station_service_events - service events in the queue
- *
- * Get an event from the station event queue (if any); attempt to service
- * the event; if event serviced, get the next event (if any) on the event
- * queue; if event not service, re-queue the event on the event queue and
- * attempt to service the next event; when serviced all events in queue,
- * finished; if don't transition to different state, just service all
- * events once; if transition to new state, service all events again.
- * Caller must hold llc_main_station.ev_q.lock.
- */
-static void llc_station_service_events(void)
-{
- struct sk_buff *skb;
-
- while ((skb = skb_dequeue(&llc_main_station.ev_q.list)) != NULL)
- llc_station_next_state(skb);
-}
-
-/**
- * llc_station_state_process - queue event and try to process queue.
- * @skb: Address of the event
- *
- * Queues an event (on the station event queue) for handling by the
- * station state machine and attempts to process any queued-up events.
- */
-static void llc_station_state_process(struct sk_buff *skb)
-{
- spin_lock_bh(&llc_main_station.ev_q.lock);
- skb_queue_tail(&llc_main_station.ev_q.list, skb);
- llc_station_service_events();
- spin_unlock_bh(&llc_main_station.ev_q.lock);
-}
-
-/**
* llc_station_rcv - send received pdu to the station state machine
* @skb: received frame.
*
@@ -296,14 +218,17 @@ static void llc_station_state_process(struct sk_buff *skb)
*/
static void llc_station_rcv(struct sk_buff *skb)
{
- llc_station_state_process(skb);
+ struct llc_station_state_trans *trans;
+
+ trans = llc_find_station_trans(skb);
+ if (trans)
+ llc_exec_station_trans_actions(trans, skb);
+ kfree_skb(skb);
}
void __init llc_station_init(void)
{
skb_queue_head_init(&llc_main_station.mac_pdu_q);
- skb_queue_head_init(&llc_main_station.ev_q.list);
- spin_lock_init(&llc_main_station.ev_q.lock);
llc_set_station_handler(llc_station_rcv);
}
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* [PATCH net-next 2/6] llc2: Remove dead code for state machine
From: Ben Hutchings @ 2012-09-16 3:11 UTC (permalink / raw)
To: David S. Miller, Arnaldo Carvalho de Melo; +Cc: netdev
In-Reply-To: <1347764982.13258.207.camel@deadeye.wl.decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 17967 bytes --]
The initial state is UP and there is no way to enter the other states
as the required event type is never generated. Delete all states,
event types, and other dead code. The only thing left is handling
of the XID and TEST commands.
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
net/llc/llc_station.c | 404 ++-----------------------------------------------
1 file changed, 9 insertions(+), 395 deletions(-)
diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index e16c1b9..917d700 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -30,19 +30,12 @@
*
* SAP and connection resource manager, one per adapter.
*
- * @state: state of station
- * @xid_r_count: XID response PDU counter
* @mac_sa: MAC source address
* @sap_list: list of related SAPs
* @ev_q: events entering state mach.
* @mac_pdu_q: PDUs ready to send to MAC
*/
struct llc_station {
- u8 state;
- u8 xid_r_count;
- struct timer_list ack_timer;
- u8 retry_count;
- u8 maximum_retry;
struct {
struct sk_buff_head list;
spinlock_t lock;
@@ -50,162 +43,38 @@ struct llc_station {
struct sk_buff_head mac_pdu_q;
};
-#define LLC_STATION_ACK_TIME (3 * HZ)
-
-int sysctl_llc_station_ack_timeout = LLC_STATION_ACK_TIME;
-
-/* Types of events (possible values in 'ev->type') */
-#define LLC_STATION_EV_TYPE_SIMPLE 1
-#define LLC_STATION_EV_TYPE_CONDITION 2
-#define LLC_STATION_EV_TYPE_PRIM 3
-#define LLC_STATION_EV_TYPE_PDU 4 /* command/response PDU */
-#define LLC_STATION_EV_TYPE_ACK_TMR 5
-#define LLC_STATION_EV_TYPE_RPT_STATUS 6
-
-/* Events */
-#define LLC_STATION_EV_ENABLE_WITH_DUP_ADDR_CHECK 1
-#define LLC_STATION_EV_ENABLE_WITHOUT_DUP_ADDR_CHECK 2
-#define LLC_STATION_EV_ACK_TMR_EXP_LT_RETRY_CNT_MAX_RETRY 3
-#define LLC_STATION_EV_ACK_TMR_EXP_EQ_RETRY_CNT_MAX_RETRY 4
-#define LLC_STATION_EV_RX_NULL_DSAP_XID_C 5
-#define LLC_STATION_EV_RX_NULL_DSAP_0_XID_R_XID_R_CNT_EQ 6
-#define LLC_STATION_EV_RX_NULL_DSAP_1_XID_R_XID_R_CNT_EQ 7
-#define LLC_STATION_EV_RX_NULL_DSAP_TEST_C 8
-#define LLC_STATION_EV_DISABLE_REQ 9
-
-struct llc_station_state_ev {
- u8 type;
- u8 prim;
- u8 prim_type;
- u8 reason;
- struct list_head node; /* node in station->ev_q.list */
-};
-
-static __inline__ struct llc_station_state_ev *
- llc_station_ev(struct sk_buff *skb)
-{
- return (struct llc_station_state_ev *)skb->cb;
-}
-
typedef int (*llc_station_ev_t)(struct sk_buff *skb);
-#define LLC_STATION_STATE_DOWN 1 /* initial state */
-#define LLC_STATION_STATE_DUP_ADDR_CHK 2
-#define LLC_STATION_STATE_UP 3
-
-#define LLC_NBR_STATION_STATES 3 /* size of state table */
-
typedef int (*llc_station_action_t)(struct sk_buff *skb);
/* Station component state table structure */
struct llc_station_state_trans {
llc_station_ev_t ev;
- u8 next_state;
llc_station_action_t *ev_actions;
};
-struct llc_station_state {
- u8 curr_state;
- struct llc_station_state_trans **transitions;
-};
-
static struct llc_station llc_main_station;
-static int llc_stat_ev_enable_with_dup_addr_check(struct sk_buff *skb)
-{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
-
- return ev->type == LLC_STATION_EV_TYPE_SIMPLE &&
- ev->prim_type ==
- LLC_STATION_EV_ENABLE_WITH_DUP_ADDR_CHECK ? 0 : 1;
-}
-
-static int llc_stat_ev_enable_without_dup_addr_check(struct sk_buff *skb)
-{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
-
- return ev->type == LLC_STATION_EV_TYPE_SIMPLE &&
- ev->prim_type ==
- LLC_STATION_EV_ENABLE_WITHOUT_DUP_ADDR_CHECK ? 0 : 1;
-}
-
-static int llc_stat_ev_ack_tmr_exp_lt_retry_cnt_max_retry(struct sk_buff *skb)
-{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
-
- return ev->type == LLC_STATION_EV_TYPE_ACK_TMR &&
- llc_main_station.retry_count <
- llc_main_station.maximum_retry ? 0 : 1;
-}
-
-static int llc_stat_ev_ack_tmr_exp_eq_retry_cnt_max_retry(struct sk_buff *skb)
-{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
-
- return ev->type == LLC_STATION_EV_TYPE_ACK_TMR &&
- llc_main_station.retry_count ==
- llc_main_station.maximum_retry ? 0 : 1;
-}
-
static int llc_stat_ev_rx_null_dsap_xid_c(struct sk_buff *skb)
{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
- return ev->type == LLC_STATION_EV_TYPE_PDU &&
- LLC_PDU_IS_CMD(pdu) && /* command PDU */
+ return LLC_PDU_IS_CMD(pdu) && /* command PDU */
LLC_PDU_TYPE_IS_U(pdu) && /* U type PDU */
LLC_U_PDU_CMD(pdu) == LLC_1_PDU_CMD_XID &&
!pdu->dsap ? 0 : 1; /* NULL DSAP value */
}
-static int llc_stat_ev_rx_null_dsap_0_xid_r_xid_r_cnt_eq(struct sk_buff *skb)
-{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
- struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
-
- return ev->type == LLC_STATION_EV_TYPE_PDU &&
- LLC_PDU_IS_RSP(pdu) && /* response PDU */
- LLC_PDU_TYPE_IS_U(pdu) && /* U type PDU */
- LLC_U_PDU_RSP(pdu) == LLC_1_PDU_CMD_XID &&
- !pdu->dsap && /* NULL DSAP value */
- !llc_main_station.xid_r_count ? 0 : 1;
-}
-
-static int llc_stat_ev_rx_null_dsap_1_xid_r_xid_r_cnt_eq(struct sk_buff *skb)
-{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
- struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
-
- return ev->type == LLC_STATION_EV_TYPE_PDU &&
- LLC_PDU_IS_RSP(pdu) && /* response PDU */
- LLC_PDU_TYPE_IS_U(pdu) && /* U type PDU */
- LLC_U_PDU_RSP(pdu) == LLC_1_PDU_CMD_XID &&
- !pdu->dsap && /* NULL DSAP value */
- llc_main_station.xid_r_count == 1 ? 0 : 1;
-}
-
static int llc_stat_ev_rx_null_dsap_test_c(struct sk_buff *skb)
{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
struct llc_pdu_un *pdu = llc_pdu_un_hdr(skb);
- return ev->type == LLC_STATION_EV_TYPE_PDU &&
- LLC_PDU_IS_CMD(pdu) && /* command PDU */
+ return LLC_PDU_IS_CMD(pdu) && /* command PDU */
LLC_PDU_TYPE_IS_U(pdu) && /* U type PDU */
LLC_U_PDU_CMD(pdu) == LLC_1_PDU_CMD_TEST &&
!pdu->dsap ? 0 : 1; /* NULL DSAP */
}
-static int llc_stat_ev_disable_req(struct sk_buff *skb)
-{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
-
- return ev->type == LLC_STATION_EV_TYPE_PRIM &&
- ev->prim == LLC_DISABLE_PRIM &&
- ev->prim_type == LLC_PRIM_TYPE_REQ ? 0 : 1;
-}
-
/**
* llc_station_send_pdu - queues PDU to send
* @skb: Address of the PDU
@@ -220,58 +89,6 @@ static void llc_station_send_pdu(struct sk_buff *skb)
break;
}
-static int llc_station_ac_start_ack_timer(struct sk_buff *skb)
-{
- mod_timer(&llc_main_station.ack_timer,
- jiffies + sysctl_llc_station_ack_timeout);
- return 0;
-}
-
-static int llc_station_ac_set_retry_cnt_0(struct sk_buff *skb)
-{
- llc_main_station.retry_count = 0;
- return 0;
-}
-
-static int llc_station_ac_inc_retry_cnt_by_1(struct sk_buff *skb)
-{
- llc_main_station.retry_count++;
- return 0;
-}
-
-static int llc_station_ac_set_xid_r_cnt_0(struct sk_buff *skb)
-{
- llc_main_station.xid_r_count = 0;
- return 0;
-}
-
-static int llc_station_ac_inc_xid_r_cnt_by_1(struct sk_buff *skb)
-{
- llc_main_station.xid_r_count++;
- return 0;
-}
-
-static int llc_station_ac_send_null_dsap_xid_c(struct sk_buff *skb)
-{
- int rc = 1;
- struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U,
- sizeof(struct llc_xid_info));
-
- if (!nskb)
- goto out;
- llc_pdu_header_init(nskb, LLC_PDU_TYPE_U, 0, 0, LLC_PDU_CMD);
- llc_pdu_init_as_xid_cmd(nskb, LLC_XID_NULL_CLASS_2, 127);
- rc = llc_mac_hdr_init(nskb, skb->dev->dev_addr, skb->dev->dev_addr);
- if (unlikely(rc))
- goto free;
- llc_station_send_pdu(nskb);
-out:
- return rc;
-free:
- kfree_skb(nskb);
- goto out;
-}
-
static int llc_station_ac_send_xid_r(struct sk_buff *skb)
{
u8 mac_da[ETH_ALEN], dsap;
@@ -326,60 +143,6 @@ free:
goto out;
}
-static int llc_station_ac_report_status(struct sk_buff *skb)
-{
- return 0;
-}
-
-/* DOWN STATE transitions */
-
-/* state transition for LLC_STATION_EV_ENABLE_WITH_DUP_ADDR_CHECK event */
-static llc_station_action_t llc_stat_down_state_actions_1[] = {
- [0] = llc_station_ac_start_ack_timer,
- [1] = llc_station_ac_set_retry_cnt_0,
- [2] = llc_station_ac_set_xid_r_cnt_0,
- [3] = llc_station_ac_send_null_dsap_xid_c,
- [4] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_down_state_trans_1 = {
- .ev = llc_stat_ev_enable_with_dup_addr_check,
- .next_state = LLC_STATION_STATE_DUP_ADDR_CHK,
- .ev_actions = llc_stat_down_state_actions_1,
-};
-
-/* state transition for LLC_STATION_EV_ENABLE_WITHOUT_DUP_ADDR_CHECK event */
-static llc_station_action_t llc_stat_down_state_actions_2[] = {
- [0] = llc_station_ac_report_status, /* STATION UP */
- [1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_down_state_trans_2 = {
- .ev = llc_stat_ev_enable_without_dup_addr_check,
- .next_state = LLC_STATION_STATE_UP,
- .ev_actions = llc_stat_down_state_actions_2,
-};
-
-/* array of pointers; one to each transition */
-static struct llc_station_state_trans *llc_stat_dwn_state_trans[] = {
- [0] = &llc_stat_down_state_trans_1,
- [1] = &llc_stat_down_state_trans_2,
- [2] = NULL,
-};
-
-/* UP STATE transitions */
-/* state transition for LLC_STATION_EV_DISABLE_REQ event */
-static llc_station_action_t llc_stat_up_state_actions_1[] = {
- [0] = llc_station_ac_report_status, /* STATION DOWN */
- [1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_up_state_trans_1 = {
- .ev = llc_stat_ev_disable_req,
- .next_state = LLC_STATION_STATE_DOWN,
- .ev_actions = llc_stat_up_state_actions_1,
-};
-
/* state transition for LLC_STATION_EV_RX_NULL_DSAP_XID_C event */
static llc_station_action_t llc_stat_up_state_actions_2[] = {
[0] = llc_station_ac_send_xid_r,
@@ -388,7 +151,6 @@ static llc_station_action_t llc_stat_up_state_actions_2[] = {
static struct llc_station_state_trans llc_stat_up_state_trans_2 = {
.ev = llc_stat_ev_rx_null_dsap_xid_c,
- .next_state = LLC_STATION_STATE_UP,
.ev_actions = llc_stat_up_state_actions_2,
};
@@ -400,127 +162,14 @@ static llc_station_action_t llc_stat_up_state_actions_3[] = {
static struct llc_station_state_trans llc_stat_up_state_trans_3 = {
.ev = llc_stat_ev_rx_null_dsap_test_c,
- .next_state = LLC_STATION_STATE_UP,
.ev_actions = llc_stat_up_state_actions_3,
};
/* array of pointers; one to each transition */
static struct llc_station_state_trans *llc_stat_up_state_trans [] = {
- [0] = &llc_stat_up_state_trans_1,
- [1] = &llc_stat_up_state_trans_2,
- [2] = &llc_stat_up_state_trans_3,
- [3] = NULL,
-};
-
-/* DUP ADDR CHK STATE transitions */
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_0_XID_R_XID_R_CNT_EQ
- * event
- */
-static llc_station_action_t llc_stat_dupaddr_state_actions_1[] = {
- [0] = llc_station_ac_inc_xid_r_cnt_by_1,
- [1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_1 = {
- .ev = llc_stat_ev_rx_null_dsap_0_xid_r_xid_r_cnt_eq,
- .next_state = LLC_STATION_STATE_DUP_ADDR_CHK,
- .ev_actions = llc_stat_dupaddr_state_actions_1,
-};
-
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_1_XID_R_XID_R_CNT_EQ
- * event
- */
-static llc_station_action_t llc_stat_dupaddr_state_actions_2[] = {
- [0] = llc_station_ac_report_status, /* DUPLICATE ADDRESS FOUND */
- [1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_2 = {
- .ev = llc_stat_ev_rx_null_dsap_1_xid_r_xid_r_cnt_eq,
- .next_state = LLC_STATION_STATE_DOWN,
- .ev_actions = llc_stat_dupaddr_state_actions_2,
-};
-
-/* state transition for LLC_STATION_EV_RX_NULL_DSAP_XID_C event */
-static llc_station_action_t llc_stat_dupaddr_state_actions_3[] = {
- [0] = llc_station_ac_send_xid_r,
- [1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_3 = {
- .ev = llc_stat_ev_rx_null_dsap_xid_c,
- .next_state = LLC_STATION_STATE_DUP_ADDR_CHK,
- .ev_actions = llc_stat_dupaddr_state_actions_3,
-};
-
-/* state transition for LLC_STATION_EV_ACK_TMR_EXP_LT_RETRY_CNT_MAX_RETRY
- * event
- */
-static llc_station_action_t llc_stat_dupaddr_state_actions_4[] = {
- [0] = llc_station_ac_start_ack_timer,
- [1] = llc_station_ac_inc_retry_cnt_by_1,
- [2] = llc_station_ac_set_xid_r_cnt_0,
- [3] = llc_station_ac_send_null_dsap_xid_c,
- [4] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_4 = {
- .ev = llc_stat_ev_ack_tmr_exp_lt_retry_cnt_max_retry,
- .next_state = LLC_STATION_STATE_DUP_ADDR_CHK,
- .ev_actions = llc_stat_dupaddr_state_actions_4,
-};
-
-/* state transition for LLC_STATION_EV_ACK_TMR_EXP_EQ_RETRY_CNT_MAX_RETRY
- * event
- */
-static llc_station_action_t llc_stat_dupaddr_state_actions_5[] = {
- [0] = llc_station_ac_report_status, /* STATION UP */
- [1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_5 = {
- .ev = llc_stat_ev_ack_tmr_exp_eq_retry_cnt_max_retry,
- .next_state = LLC_STATION_STATE_UP,
- .ev_actions = llc_stat_dupaddr_state_actions_5,
-};
-
-/* state transition for LLC_STATION_EV_DISABLE_REQ event */
-static llc_station_action_t llc_stat_dupaddr_state_actions_6[] = {
- [0] = llc_station_ac_report_status, /* STATION DOWN */
- [1] = NULL,
-};
-
-static struct llc_station_state_trans llc_stat_dupaddr_state_trans_6 = {
- .ev = llc_stat_ev_disable_req,
- .next_state = LLC_STATION_STATE_DOWN,
- .ev_actions = llc_stat_dupaddr_state_actions_6,
-};
-
-/* array of pointers; one to each transition */
-static struct llc_station_state_trans *llc_stat_dupaddr_state_trans[] = {
- [0] = &llc_stat_dupaddr_state_trans_6, /* Request */
- [1] = &llc_stat_dupaddr_state_trans_4, /* Timer */
- [2] = &llc_stat_dupaddr_state_trans_5,
- [3] = &llc_stat_dupaddr_state_trans_1, /* Receive frame */
- [4] = &llc_stat_dupaddr_state_trans_2,
- [5] = &llc_stat_dupaddr_state_trans_3,
- [6] = NULL,
-};
-
-static struct llc_station_state
- llc_station_state_table[LLC_NBR_STATION_STATES] = {
- [LLC_STATION_STATE_DOWN - 1] = {
- .curr_state = LLC_STATION_STATE_DOWN,
- .transitions = llc_stat_dwn_state_trans,
- },
- [LLC_STATION_STATE_DUP_ADDR_CHK - 1] = {
- .curr_state = LLC_STATION_STATE_DUP_ADDR_CHK,
- .transitions = llc_stat_dupaddr_state_trans,
- },
- [LLC_STATION_STATE_UP - 1] = {
- .curr_state = LLC_STATION_STATE_UP,
- .transitions = llc_stat_up_state_trans,
- },
+ &llc_stat_up_state_trans_2,
+ &llc_stat_up_state_trans_3,
+ NULL,
};
/**
@@ -557,10 +206,8 @@ static struct llc_station_state_trans *
int i = 0;
struct llc_station_state_trans *rc = NULL;
struct llc_station_state_trans **next_trans;
- struct llc_station_state *curr_state =
- &llc_station_state_table[llc_main_station.state - 1];
- for (next_trans = curr_state->transitions; next_trans[i]; i++)
+ for (next_trans = llc_stat_up_state_trans; next_trans[i]; i++)
if (!next_trans[i]->ev(skb)) {
rc = next_trans[i];
break;
@@ -576,10 +223,7 @@ static struct llc_station_state_trans *
*/
static void llc_station_free_ev(struct sk_buff *skb)
{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
-
- if (ev->type == LLC_STATION_EV_TYPE_PDU)
- kfree_skb(skb);
+ kfree_skb(skb);
}
/**
@@ -594,26 +238,18 @@ static u16 llc_station_next_state(struct sk_buff *skb)
u16 rc = 1;
struct llc_station_state_trans *trans;
- if (llc_main_station.state > LLC_NBR_STATION_STATES)
- goto out;
trans = llc_find_station_trans(skb);
- if (trans) {
+ if (trans)
/* got the state to which we next transition; perform the
* actions associated with this transition before actually
* transitioning to the next state
*/
rc = llc_exec_station_trans_actions(trans, skb);
- if (!rc)
- /* transition station to next state if all actions
- * execute successfully; done; wait for next event
- */
- llc_main_station.state = trans->next_state;
- } else
+ else
/* event not recognized in current state; re-queue it for
* processing again at a later time; return failure
*/
rc = 0;
-out:
llc_station_free_ev(skb);
return rc;
}
@@ -652,18 +288,6 @@ static void llc_station_state_process(struct sk_buff *skb)
spin_unlock_bh(&llc_main_station.ev_q.lock);
}
-static void llc_station_ack_tmr_cb(unsigned long timeout_data)
-{
- struct sk_buff *skb = alloc_skb(0, GFP_ATOMIC);
-
- if (skb) {
- struct llc_station_state_ev *ev = llc_station_ev(skb);
-
- ev->type = LLC_STATION_EV_TYPE_ACK_TMR;
- llc_station_state_process(skb);
- }
-}
-
/**
* llc_station_rcv - send received pdu to the station state machine
* @skb: received frame.
@@ -672,10 +296,6 @@ static void llc_station_ack_tmr_cb(unsigned long timeout_data)
*/
static void llc_station_rcv(struct sk_buff *skb)
{
- struct llc_station_state_ev *ev = llc_station_ev(skb);
-
- ev->type = LLC_STATION_EV_TYPE_PDU;
- ev->reason = 0;
llc_station_state_process(skb);
}
@@ -684,12 +304,6 @@ void __init llc_station_init(void)
skb_queue_head_init(&llc_main_station.mac_pdu_q);
skb_queue_head_init(&llc_main_station.ev_q.list);
spin_lock_init(&llc_main_station.ev_q.lock);
- setup_timer(&llc_main_station.ack_timer, llc_station_ack_tmr_cb,
- (unsigned long)&llc_main_station);
- llc_main_station.ack_timer.expires = jiffies +
- sysctl_llc_station_ack_timeout;
- llc_main_station.maximum_retry = 1;
- llc_main_station.state = LLC_STATION_STATE_UP;
llc_set_station_handler(llc_station_rcv);
}
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* [PATCH net-next 1/6] llc2: Remove pointless indirection through llc_stat_state_trans_end
From: Ben Hutchings @ 2012-09-16 3:10 UTC (permalink / raw)
To: David S. Miller, Arnaldo Carvalho de Melo; +Cc: netdev
In-Reply-To: <1347764982.13258.207.camel@deadeye.wl.decadent.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
net/llc/llc_station.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c
index b2f2bac..e16c1b9 100644
--- a/net/llc/llc_station.c
+++ b/net/llc/llc_station.c
@@ -331,14 +331,6 @@ static int llc_station_ac_report_status(struct sk_buff *skb)
return 0;
}
-/* COMMON STATION STATE transitions */
-
-/* dummy last-transition indicator; common to all state transition groups
- * last entry for this state
- * all members are zeros, .bss zeroes it
- */
-static struct llc_station_state_trans llc_stat_state_trans_end;
-
/* DOWN STATE transitions */
/* state transition for LLC_STATION_EV_ENABLE_WITH_DUP_ADDR_CHECK event */
@@ -372,7 +364,7 @@ static struct llc_station_state_trans llc_stat_down_state_trans_2 = {
static struct llc_station_state_trans *llc_stat_dwn_state_trans[] = {
[0] = &llc_stat_down_state_trans_1,
[1] = &llc_stat_down_state_trans_2,
- [2] = &llc_stat_state_trans_end,
+ [2] = NULL,
};
/* UP STATE transitions */
@@ -417,7 +409,7 @@ static struct llc_station_state_trans *llc_stat_up_state_trans [] = {
[0] = &llc_stat_up_state_trans_1,
[1] = &llc_stat_up_state_trans_2,
[2] = &llc_stat_up_state_trans_3,
- [3] = &llc_stat_state_trans_end,
+ [3] = NULL,
};
/* DUP ADDR CHK STATE transitions */
@@ -512,7 +504,7 @@ static struct llc_station_state_trans *llc_stat_dupaddr_state_trans[] = {
[3] = &llc_stat_dupaddr_state_trans_1, /* Receive frame */
[4] = &llc_stat_dupaddr_state_trans_2,
[5] = &llc_stat_dupaddr_state_trans_3,
- [6] = &llc_stat_state_trans_end,
+ [6] = NULL,
};
static struct llc_station_state
@@ -568,7 +560,7 @@ static struct llc_station_state_trans *
struct llc_station_state *curr_state =
&llc_station_state_table[llc_main_station.state - 1];
- for (next_trans = curr_state->transitions; next_trans[i]->ev; i++)
+ for (next_trans = curr_state->transitions; next_trans[i]; i++)
if (!next_trans[i]->ev(skb)) {
rc = next_trans[i];
break;
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* [PATCH 0/6] llc2: Simplify llc_station
From: Ben Hutchings @ 2012-09-16 3:09 UTC (permalink / raw)
To: David S. Miller, Arnaldo Carvalho de Melo; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 742 bytes --]
There seem to have been some grand plans for llc_station, but as they
haven't been fulfilled it's just unnecessarily complicated.
Ben.
Ben Hutchings (6):
llc2: Remove pointless indirection through llc_stat_state_trans_end
llc2: Remove dead code for state machine
llc2: Collapse the station event receive path
llc2: Remove the station send queue
llc2: Remove explicit indexing of state action arrays
llc2: Collapse remainder of state machine into simple if-else
if-statement
net/llc/llc_station.c | 600 +------------------------------------------------
1 file changed, 9 insertions(+), 591 deletions(-)
--
Ben Hutchings
Experience is what causes a person to make new mistakes instead of old ones.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [net] e1000: Small packets may get corrupted during padding by HW
From: John Fastabend @ 2012-09-16 2:30 UTC (permalink / raw)
To: Dave, Tushar N
Cc: Michal Miroslaw, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <50552FF1.5030708@intel.com>
On 9/15/2012 6:48 PM, John Fastabend wrote:
> On 9/15/2012 6:25 PM, Dave, Tushar N wrote:
>>> -----Original Message-----
>>> From: Michał Mirosław [mailto:mirqus@gmail.com]
>>> Sent: Saturday, September 15, 2012 1:45 PM
>>> To: Kirsher, Jeffrey T
>>> Cc: davem@davemloft.net; Dave, Tushar N; netdev@vger.kernel.org;
>>> gospo@redhat.com; sassmann@redhat.com
>>> Subject: Re: [net] e1000: Small packets may get corrupted during padding
>>> by HW
>>>
>>> 2012/9/15 Jeff Kirsher <jeffrey.t.kirsher@intel.com>:
>>>> From: Tushar Dave <tushar.n.dave@intel.com>
>>>>
>>>> On PCI/PCI-X HW, if packet size is less than ETH_ZLEN, packets may get
>>>> corrupted during padding by HW.
>>>> To WA this issue, pad all small packets manually.
>>>>
>>>> Signed-off-by: Tushar Dave <tushar.n.dave@intel.com>
>>>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>>> ---
>>>> drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c
>>>> b/drivers/net/ethernet/intel/e1000/e1000_main.c
>>>> index 3bfbb8d..bde337e 100644
>>>> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
>>>> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
>>>> @@ -3149,6 +3149,17 @@ static netdev_tx_t e1000_xmit_frame(struct
>>> sk_buff *skb,
>>>> return NETDEV_TX_OK;
>>>> }
>>>>
>>>> + /* On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
>>>> + * packets may get corrupted during padding by HW.
>>>> + * To WA this issue, pad all small packets manually.
>>>> + */
>>>> + if (skb->len < ETH_ZLEN) {
>>>> + if (skb_pad(skb, ETH_ZLEN - skb->len))
>>>> + return NETDEV_TX_OK;
>>>> + skb->len = ETH_ZLEN;
>>>> + skb_set_tail_pointer(skb, ETH_ZLEN);
>>>> + }
>>>> +
>>>
>>> Isn't there a skb_padto() that does just this?
>>
>> Skb_padto calls skb_pad(). Calling skb_pad directly saves some cycles.
>>
>
> How/where?
>
OK maybe you avoid an if case.
^ permalink raw reply
* [PATCH] fix ZOMBIE state bug in PPPOE driver
From: Xiaodong Xu @ 2012-09-16 2:30 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev
In-Reply-To: <CANEcBPSEamgOiGqNUSwB+hYCo1k2OTXDA=qsrw0Co8_F7treWw@mail.gmail.com>
Hi All,
I found a bug in kernel PPPOE driver.
When PPPOE is running over a virtual ethernet interface (e.g., a
bonding interface) and the user tries to delete the interface in case
the PPPOE state is ZOMBIE, the kernel will loop infinitely while
unregistering net_device for the reference count is not reset to zero
which should be done by dev_put().
The following patch could fix this issue:
$ git diff
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index cbf7047..20f31d0 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -570,7 +570,7 @@ static int pppoe_release(struct socket *sock)
po = pppox_sk(sk);
- if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND)) {
+ if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND | PPPOX_ZOMBIE)) {
dev_put(po->pppoe_dev);
po->pppoe_dev = NULL;
}
Thanks.
Regards,
Xiaodong Xu
^ permalink raw reply related
* Re: [net] e1000: Small packets may get corrupted during padding by HW
From: John Fastabend @ 2012-09-16 1:48 UTC (permalink / raw)
To: Dave, Tushar N
Cc: Michal Miroslaw, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <061C8A8601E8EE4CA8D8FD6990CEA89130DC20AA@ORSMSX102.amr.corp.intel.com>
On 9/15/2012 6:25 PM, Dave, Tushar N wrote:
>> -----Original Message-----
>> From: Michał Mirosław [mailto:mirqus@gmail.com]
>> Sent: Saturday, September 15, 2012 1:45 PM
>> To: Kirsher, Jeffrey T
>> Cc: davem@davemloft.net; Dave, Tushar N; netdev@vger.kernel.org;
>> gospo@redhat.com; sassmann@redhat.com
>> Subject: Re: [net] e1000: Small packets may get corrupted during padding
>> by HW
>>
>> 2012/9/15 Jeff Kirsher <jeffrey.t.kirsher@intel.com>:
>>> From: Tushar Dave <tushar.n.dave@intel.com>
>>>
>>> On PCI/PCI-X HW, if packet size is less than ETH_ZLEN, packets may get
>>> corrupted during padding by HW.
>>> To WA this issue, pad all small packets manually.
>>>
>>> Signed-off-by: Tushar Dave <tushar.n.dave@intel.com>
>>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> ---
>>> drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c
>>> b/drivers/net/ethernet/intel/e1000/e1000_main.c
>>> index 3bfbb8d..bde337e 100644
>>> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
>>> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
>>> @@ -3149,6 +3149,17 @@ static netdev_tx_t e1000_xmit_frame(struct
>> sk_buff *skb,
>>> return NETDEV_TX_OK;
>>> }
>>>
>>> + /* On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
>>> + * packets may get corrupted during padding by HW.
>>> + * To WA this issue, pad all small packets manually.
>>> + */
>>> + if (skb->len < ETH_ZLEN) {
>>> + if (skb_pad(skb, ETH_ZLEN - skb->len))
>>> + return NETDEV_TX_OK;
>>> + skb->len = ETH_ZLEN;
>>> + skb_set_tail_pointer(skb, ETH_ZLEN);
>>> + }
>>> +
>>
>> Isn't there a skb_padto() that does just this?
>
> Skb_padto calls skb_pad(). Calling skb_pad directly saves some cycles.
>
How/where?
static inline int skb_padto(struct sk_buff *skb, unsigned int len)
{
unsigned int size = skb->len;
if (likely(size >= len))
return 0;
return skb_pad(skb, len - size);
}
Also wouldn't you want an unlikely() in your patch?
.John
^ permalink raw reply
* RE: [net] e1000: Small packets may get corrupted during padding by HW
From: Dave, Tushar N @ 2012-09-16 1:25 UTC (permalink / raw)
To: Michal Miroslaw, Kirsher, Jeffrey T
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
sassmann@redhat.com
In-Reply-To: <CAHXqBFJZnGUSUutN=L-_LpCN7dXLPFY0ndJnQJ10NCcbzPVLww@mail.gmail.com>
>-----Original Message-----
>From: Michał Mirosław [mailto:mirqus@gmail.com]
>Sent: Saturday, September 15, 2012 1:45 PM
>To: Kirsher, Jeffrey T
>Cc: davem@davemloft.net; Dave, Tushar N; netdev@vger.kernel.org;
>gospo@redhat.com; sassmann@redhat.com
>Subject: Re: [net] e1000: Small packets may get corrupted during padding
>by HW
>
>2012/9/15 Jeff Kirsher <jeffrey.t.kirsher@intel.com>:
>> From: Tushar Dave <tushar.n.dave@intel.com>
>>
>> On PCI/PCI-X HW, if packet size is less than ETH_ZLEN, packets may get
>> corrupted during padding by HW.
>> To WA this issue, pad all small packets manually.
>>
>> Signed-off-by: Tushar Dave <tushar.n.dave@intel.com>
>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>> drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c
>> b/drivers/net/ethernet/intel/e1000/e1000_main.c
>> index 3bfbb8d..bde337e 100644
>> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
>> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
>> @@ -3149,6 +3149,17 @@ static netdev_tx_t e1000_xmit_frame(struct
>sk_buff *skb,
>> return NETDEV_TX_OK;
>> }
>>
>> + /* On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
>> + * packets may get corrupted during padding by HW.
>> + * To WA this issue, pad all small packets manually.
>> + */
>> + if (skb->len < ETH_ZLEN) {
>> + if (skb_pad(skb, ETH_ZLEN - skb->len))
>> + return NETDEV_TX_OK;
>> + skb->len = ETH_ZLEN;
>> + skb_set_tail_pointer(skb, ETH_ZLEN);
>> + }
>> +
>
>Isn't there a skb_padto() that does just this?
Skb_padto calls skb_pad(). Calling skb_pad directly saves some cycles.
^ permalink raw reply
* Uppgradera ditt e-postkonto
From: E-post administratör @ 2012-09-14 8:11 UTC (permalink / raw)
To: info; +Cc: info
Kära e-postanvändare,
Din webbmail kvot har överskridit den fastställda kvoten som är 3GB. du
för närvarande kör på 3.9GB.
För att åter aktivera och öka din webbmail kvot Klicka på länken
nedan:
https://docs.google.com/spreadsheet/viewform?formkey=dHkwbU1HTU1EcUVrM25YZHJweFlqUkE6MQ
Underlåtenhet att göra detta kan resultera i uppsägning av din webbmail
konto.
Tack, och ber om ursäkt för besväret
Localhost.
^ permalink raw reply
* Re: [net] e1000: Small packets may get corrupted during padding by HW
From: Michał Mirosław @ 2012-09-15 20:44 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Tushar Dave, netdev, gospo, sassmann
In-Reply-To: <1347740217-10257-1-git-send-email-jeffrey.t.kirsher@intel.com>
2012/9/15 Jeff Kirsher <jeffrey.t.kirsher@intel.com>:
> From: Tushar Dave <tushar.n.dave@intel.com>
>
> On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
> packets may get corrupted during padding by HW.
> To WA this issue, pad all small packets manually.
>
> Signed-off-by: Tushar Dave <tushar.n.dave@intel.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
> index 3bfbb8d..bde337e 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -3149,6 +3149,17 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
> return NETDEV_TX_OK;
> }
>
> + /* On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
> + * packets may get corrupted during padding by HW.
> + * To WA this issue, pad all small packets manually.
> + */
> + if (skb->len < ETH_ZLEN) {
> + if (skb_pad(skb, ETH_ZLEN - skb->len))
> + return NETDEV_TX_OK;
> + skb->len = ETH_ZLEN;
> + skb_set_tail_pointer(skb, ETH_ZLEN);
> + }
> +
Isn't there a skb_padto() that does just this?
Best Regards,
Michał Mirosław
^ permalink raw reply
* [net] e1000: Small packets may get corrupted during padding by HW
From: Jeff Kirsher @ 2012-09-15 20:16 UTC (permalink / raw)
To: davem; +Cc: Tushar Dave, netdev, gospo, sassmann, Jeff Kirsher
From: Tushar Dave <tushar.n.dave@intel.com>
On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
packets may get corrupted during padding by HW.
To WA this issue, pad all small packets manually.
Signed-off-by: Tushar Dave <tushar.n.dave@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000/e1000_main.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 3bfbb8d..bde337e 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -3149,6 +3149,17 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
return NETDEV_TX_OK;
}
+ /* On PCI/PCI-X HW, if packet size is less than ETH_ZLEN,
+ * packets may get corrupted during padding by HW.
+ * To WA this issue, pad all small packets manually.
+ */
+ if (skb->len < ETH_ZLEN) {
+ if (skb_pad(skb, ETH_ZLEN - skb->len))
+ return NETDEV_TX_OK;
+ skb->len = ETH_ZLEN;
+ skb_set_tail_pointer(skb, ETH_ZLEN);
+ }
+
mss = skb_shinfo(skb)->gso_size;
/* The controller does a simple calculation to
* make sure there is enough room in the FIFO before
--
1.7.11.4
^ permalink raw reply related
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