* [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 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 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 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
* 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
* 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
* [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
* 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
* 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
* 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: [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
* 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
* 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
* [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
* 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
* Re: [PATCH] fix ZOMBIE state bug in PPPOE driver
From: Cong Wang @ 2012-09-17 3:35 UTC (permalink / raw)
To: Xiaodong Xu; +Cc: linux-kernel, netdev
In-Reply-To: <CANEcBPQnsV26UkyGxms0vKM9wK1NxfKjN4Z0qGipQtV4UPZgGA@mail.gmail.com>
On Sun, Sep 16, 2012 at 10:30 AM, Xiaodong Xu <stid.smth@gmail.com> wrote:
> 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:
You missed your Signed-off-by, please read
Documentation/SubmittingPatches and check your patch with
scripts/checkpatch.pl before sending.
^ permalink raw reply
* [net-next 0/8][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann
This series contains updates to e1000 and ixgbe. Most notably is
the added debugfs support in ixgbe.
The following are changes since commit 7f2e6a5d8608d0353b017a0fe15502307593734e:
drivers/isdn/gigaset/common.c: Remove useless kfree
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Alexander Duyck (2):
ixgbe: Fix ordering of things so that PF correctly configures its
VLANs
ixgbe: Fix VF rate limiting to correctly account for more queues per
VF
Catherine Sullivan (3):
ixgbe: add debugfs support
ixgbe: added netdev_ops file to debugfs
ixgbe: added reg_ops file to debugfs
Emil Tantilov (1):
ixgbe: fix reporting of spoofed packets
Mark Rustad (1):
ixgbe: Improve statistics accuracy for DDP traffic
Otto Estuardo Solares Cabrera (1):
e1000: add byte queue limits
drivers/net/ethernet/intel/e1000/e1000_main.c | 10 +
drivers/net/ethernet/intel/ixgbe/Makefile | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 10 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 300 +++++++++++++++++++++++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 60 +++--
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 105 ++++----
6 files changed, 423 insertions(+), 64 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
--
1.7.11.4
^ permalink raw reply
* [net-next 1/8] e1000: add byte queue limits
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Otto Estuardo Solares Cabrera, netdev, gospo, sassmann,
Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Otto Estuardo Solares Cabrera <solca@galileo.edu>
Signed-off-by: Otto Estuardo Solares Cabrera <solca@galileo.edu>
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 | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 0ae2fcf..3a8368e 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -2014,6 +2014,7 @@ static void e1000_clean_tx_ring(struct e1000_adapter *adapter,
e1000_unmap_and_free_tx_resource(adapter, buffer_info);
}
+ netdev_reset_queue(adapter->netdev);
size = sizeof(struct e1000_buffer) * tx_ring->count;
memset(tx_ring->buffer_info, 0, size);
@@ -3262,6 +3263,7 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
nr_frags, mss);
if (count) {
+ netdev_sent_queue(netdev, skb->len);
skb_tx_timestamp(skb);
e1000_tx_queue(adapter, tx_ring, tx_flags, count);
@@ -3849,6 +3851,7 @@ static bool e1000_clean_tx_irq(struct e1000_adapter *adapter,
unsigned int i, eop;
unsigned int count = 0;
unsigned int total_tx_bytes=0, total_tx_packets=0;
+ unsigned int bytes_compl = 0, pkts_compl = 0;
i = tx_ring->next_to_clean;
eop = tx_ring->buffer_info[i].next_to_watch;
@@ -3866,6 +3869,11 @@ static bool e1000_clean_tx_irq(struct e1000_adapter *adapter,
if (cleaned) {
total_tx_packets += buffer_info->segs;
total_tx_bytes += buffer_info->bytecount;
+ if (buffer_info->skb) {
+ bytes_compl += buffer_info->skb->len;
+ pkts_compl++;
+ }
+
}
e1000_unmap_and_free_tx_resource(adapter, buffer_info);
tx_desc->upper.data = 0;
@@ -3879,6 +3887,8 @@ static bool e1000_clean_tx_irq(struct e1000_adapter *adapter,
tx_ring->next_to_clean = i;
+ netdev_completed_queue(netdev, pkts_compl, bytes_compl);
+
#define TX_WAKE_THRESHOLD 32
if (unlikely(count && netif_carrier_ok(netdev) &&
E1000_DESC_UNUSED(tx_ring) >= TX_WAKE_THRESHOLD)) {
--
1.7.11.4
^ permalink raw reply related
* [net-next 4/8] ixgbe: fix reporting of spoofed packets
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Use %u instead of %d to display u32 variable.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Tested-by: Robert Garrett <RobertX.Garrett@intel.com>
Tested-by: Robert Garrett <RobertX.Garrett@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ac91567..e641f14 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5573,7 +5573,7 @@ static void ixgbe_spoof_check(struct ixgbe_adapter *adapter)
if (!ssvpc)
return;
- e_warn(drv, "%d Spoofed packets detected\n", ssvpc);
+ e_warn(drv, "%u Spoofed packets detected\n", ssvpc);
}
/**
--
1.7.11.4
^ permalink raw reply related
* [net-next 3/8] ixgbe: Fix VF rate limiting to correctly account for more queues per VF
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change fixes the assumptions of the rate limiting code that previously
assumed that each VF would only ever have 2 queues. This update makes it
so that we now use a queues per pool value that is determined based on the
VMDq feature mask.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Acked-By: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Tested-by: Robert Garrett <RobertX.Garrett@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 100 +++++++++++++++----------
1 file changed, 59 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 3b1c914..dce48bf 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -815,9 +815,9 @@ out:
return err;
}
-static int ixgbe_link_mbps(int internal_link_speed)
+static int ixgbe_link_mbps(struct ixgbe_adapter *adapter)
{
- switch (internal_link_speed) {
+ switch (adapter->link_speed) {
case IXGBE_LINK_SPEED_100_FULL:
return 100;
case IXGBE_LINK_SPEED_1GB_FULL:
@@ -829,27 +829,30 @@ static int ixgbe_link_mbps(int internal_link_speed)
}
}
-static void ixgbe_set_vf_rate_limit(struct ixgbe_hw *hw, int vf, int tx_rate,
- int link_speed)
+static void ixgbe_set_vf_rate_limit(struct ixgbe_adapter *adapter, int vf)
{
- int rf_dec, rf_int;
- u32 bcnrc_val;
+ struct ixgbe_ring_feature *vmdq = &adapter->ring_feature[RING_F_VMDQ];
+ struct ixgbe_hw *hw = &adapter->hw;
+ u32 bcnrc_val = 0;
+ u16 queue, queues_per_pool;
+ u16 tx_rate = adapter->vfinfo[vf].tx_rate;
+
+ if (tx_rate) {
+ /* start with base link speed value */
+ bcnrc_val = adapter->vf_rate_link_speed;
- if (tx_rate != 0) {
/* Calculate the rate factor values to set */
- rf_int = link_speed / tx_rate;
- rf_dec = (link_speed - (rf_int * tx_rate));
- rf_dec = (rf_dec * (1<<IXGBE_RTTBCNRC_RF_INT_SHIFT)) / tx_rate;
-
- bcnrc_val = IXGBE_RTTBCNRC_RS_ENA;
- bcnrc_val |= ((rf_int<<IXGBE_RTTBCNRC_RF_INT_SHIFT) &
- IXGBE_RTTBCNRC_RF_INT_MASK);
- bcnrc_val |= (rf_dec & IXGBE_RTTBCNRC_RF_DEC_MASK);
- } else {
- bcnrc_val = 0;
+ bcnrc_val <<= IXGBE_RTTBCNRC_RF_INT_SHIFT;
+ bcnrc_val /= tx_rate;
+
+ /* clear everything but the rate factor */
+ bcnrc_val &= IXGBE_RTTBCNRC_RF_INT_MASK |
+ IXGBE_RTTBCNRC_RF_DEC_MASK;
+
+ /* enable the rate scheduler */
+ bcnrc_val |= IXGBE_RTTBCNRC_RS_ENA;
}
- IXGBE_WRITE_REG(hw, IXGBE_RTTDQSEL, 2*vf); /* vf Y uses queue 2*Y */
/*
* Set global transmit compensation time to the MMW_SIZE in RTTBCNRM
* register. Typically MMW_SIZE=0x014 if 9728-byte jumbo is supported
@@ -866,53 +869,68 @@ static void ixgbe_set_vf_rate_limit(struct ixgbe_hw *hw, int vf, int tx_rate,
break;
}
- IXGBE_WRITE_REG(hw, IXGBE_RTTBCNRC, bcnrc_val);
+ /* determine how many queues per pool based on VMDq mask */
+ queues_per_pool = __ALIGN_MASK(1, ~vmdq->mask);
+
+ /* write value for all Tx queues belonging to VF */
+ for (queue = 0; queue < queues_per_pool; queue++) {
+ unsigned int reg_idx = (vf * queues_per_pool) + queue;
+
+ IXGBE_WRITE_REG(hw, IXGBE_RTTDQSEL, reg_idx);
+ IXGBE_WRITE_REG(hw, IXGBE_RTTBCNRC, bcnrc_val);
+ }
}
void ixgbe_check_vf_rate_limit(struct ixgbe_adapter *adapter)
{
- int actual_link_speed, i;
- bool reset_rate = false;
+ int i;
/* VF Tx rate limit was not set */
- if (adapter->vf_rate_link_speed == 0)
+ if (!adapter->vf_rate_link_speed)
return;
- actual_link_speed = ixgbe_link_mbps(adapter->link_speed);
- if (actual_link_speed != adapter->vf_rate_link_speed) {
- reset_rate = true;
+ if (ixgbe_link_mbps(adapter) != adapter->vf_rate_link_speed) {
adapter->vf_rate_link_speed = 0;
dev_info(&adapter->pdev->dev,
- "Link speed has been changed. VF Transmit rate "
- "is disabled\n");
+ "Link speed has been changed. VF Transmit rate is disabled\n");
}
for (i = 0; i < adapter->num_vfs; i++) {
- if (reset_rate)
+ if (!adapter->vf_rate_link_speed)
adapter->vfinfo[i].tx_rate = 0;
- ixgbe_set_vf_rate_limit(&adapter->hw, i,
- adapter->vfinfo[i].tx_rate,
- actual_link_speed);
+ ixgbe_set_vf_rate_limit(adapter, i);
}
}
int ixgbe_ndo_set_vf_bw(struct net_device *netdev, int vf, int tx_rate)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
- struct ixgbe_hw *hw = &adapter->hw;
- int actual_link_speed;
+ int link_speed;
- actual_link_speed = ixgbe_link_mbps(adapter->link_speed);
- if ((vf >= adapter->num_vfs) || (!adapter->link_up) ||
- (tx_rate > actual_link_speed) || (actual_link_speed != 10000) ||
- ((tx_rate != 0) && (tx_rate <= 10)))
- /* rate limit cannot be set to 10Mb or less in 10Gb adapters */
+ /* verify VF is active */
+ if (vf >= adapter->num_vfs)
return -EINVAL;
- adapter->vf_rate_link_speed = actual_link_speed;
- adapter->vfinfo[vf].tx_rate = (u16)tx_rate;
- ixgbe_set_vf_rate_limit(hw, vf, tx_rate, actual_link_speed);
+ /* verify link is up */
+ if (!adapter->link_up)
+ return -EINVAL;
+
+ /* verify we are linked at 10Gbps */
+ link_speed = ixgbe_link_mbps(adapter);
+ if (link_speed != 10000)
+ return -EINVAL;
+
+ /* rate limit cannot be less than 10Mbs or greater than link speed */
+ if (tx_rate && ((tx_rate <= 10) || (tx_rate > link_speed)))
+ return -EINVAL;
+
+ /* store values */
+ adapter->vf_rate_link_speed = link_speed;
+ adapter->vfinfo[vf].tx_rate = tx_rate;
+
+ /* update hardware configuration */
+ ixgbe_set_vf_rate_limit(adapter, vf);
return 0;
}
--
1.7.11.4
^ permalink raw reply related
* [net-next 2/8] ixgbe: Fix ordering of things so that PF correctly configures its VLANs
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
The PF was not correctly registering any of its VLANs. As a result any
VLAN tagged traffic from the VF would not be delivered to the PF because
the VLAN was never assigned to the PF pool.
In addition the VF was not allowed to receive traffic from VLAN 0 if it was
allowed to receive untagged frames. This change corrects that so that it
will correctly receive traffic from VLAN 0.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 +++++----
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 5 +++++
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 1cbb34f..ac91567 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3660,8 +3660,6 @@ static void ixgbe_configure_dcb(struct ixgbe_adapter *adapter)
if (hw->mac.type == ixgbe_mac_82598EB)
netif_set_gso_max_size(adapter->netdev, 32768);
- hw->mac.ops.set_vfta(&adapter->hw, 0, 0, true);
-
#ifdef IXGBE_FCOE
if (adapter->netdev->features & NETIF_F_FCOE_MTU)
max_frame = max(max_frame, IXGBE_FCOE_JUMBO_FRAME_SIZE);
@@ -3861,6 +3859,11 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter)
#ifdef CONFIG_IXGBE_DCB
ixgbe_configure_dcb(adapter);
#endif
+ /*
+ * We must restore virtualization before VLANs or else
+ * the VLVF registers will not be populated
+ */
+ ixgbe_configure_virtualization(adapter);
ixgbe_set_rx_mode(adapter->netdev);
ixgbe_restore_vlan(adapter);
@@ -3892,8 +3895,6 @@ static void ixgbe_configure(struct ixgbe_adapter *adapter)
break;
}
- ixgbe_configure_virtualization(adapter);
-
#ifdef IXGBE_FCOE
/* configure FCoE L2 filters, redirection table, and Rx control */
ixgbe_configure_fcoe(adapter);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 4fea871..3b1c914 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -346,6 +346,10 @@ void ixgbe_restore_vf_multicasts(struct ixgbe_adapter *adapter)
static int ixgbe_set_vf_vlan(struct ixgbe_adapter *adapter, int add, int vid,
u32 vf)
{
+ /* VLAN 0 is a special case, don't allow it to be removed */
+ if (!vid && !add)
+ return 0;
+
return adapter->hw.mac.ops.set_vfta(&adapter->hw, vid, vf, (bool)add);
}
@@ -414,6 +418,7 @@ static inline void ixgbe_vf_reset_event(struct ixgbe_adapter *adapter, u32 vf)
VLAN_PRIO_SHIFT)), vf);
ixgbe_set_vmolr(hw, vf, false);
} else {
+ ixgbe_set_vf_vlan(adapter, true, 0, vf);
ixgbe_set_vmvir(adapter, 0, vf);
ixgbe_set_vmolr(hw, vf, true);
}
--
1.7.11.4
^ permalink raw reply related
* [net-next 5/8] ixgbe: add debugfs support
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Catherine Sullivan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Catherine Sullivan <catherine.sullivan@intel.com>
This patch adds debugfs support to the ixgbe driver to give
users the ability to access kernel information and to
simulate kernel events.
The filesystem is set up in the following driver/PCI-instance
hierarchy:
<debugfs>
|-- ixgbe
|-- PCI instance
| |-- attribute files
Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/Makefile | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 10 ++-
drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 79 ++++++++++++++++++++++++
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 17 +++++
4 files changed, 106 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile b/drivers/net/ethernet/intel/ixgbe/Makefile
index 5fd5d04..89f40e5 100644
--- a/drivers/net/ethernet/intel/ixgbe/Makefile
+++ b/drivers/net/ethernet/intel/ixgbe/Makefile
@@ -32,7 +32,7 @@
obj-$(CONFIG_IXGBE) += ixgbe.o
-ixgbe-objs := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o \
+ixgbe-objs := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o ixgbe_debugfs.o\
ixgbe_82599.o ixgbe_82598.o ixgbe_phy.o ixgbe_sriov.o \
ixgbe_mbx.o ixgbe_x540.o ixgbe_lib.o
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index bffcf1f..5bd2676 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -597,6 +597,9 @@ struct ixgbe_adapter {
#ifdef CONFIG_IXGBE_HWMON
struct hwmon_buff ixgbe_hwmon_buff;
#endif /* CONFIG_IXGBE_HWMON */
+#ifdef CONFIG_DEBUG_FS
+ struct dentry *ixgbe_dbg_adapter;
+#endif /*CONFIG_DEBUG_FS*/
};
struct ixgbe_fdir_filter {
@@ -725,7 +728,12 @@ extern int ixgbe_fcoe_get_hbainfo(struct net_device *netdev,
struct netdev_fcoe_hbainfo *info);
extern u8 ixgbe_fcoe_get_tc(struct ixgbe_adapter *adapter);
#endif /* IXGBE_FCOE */
-
+#ifdef CONFIG_DEBUG_FS
+extern void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter);
+extern void ixgbe_dbg_adapter_exit(struct ixgbe_adapter *adapter);
+extern void ixgbe_dbg_init(void);
+extern void ixgbe_dbg_exit(void);
+#endif /* CONFIG_DEBUG_FS */
static inline struct netdev_queue *txring_txq(const struct ixgbe_ring *ring)
{
return netdev_get_tx_queue(ring->netdev, ring->queue_index);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
new file mode 100644
index 0000000..0b08b6c
--- /dev/null
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -0,0 +1,79 @@
+/*******************************************************************************
+
+ Intel 10 Gigabit PCI Express Linux driver
+ Copyright(c) 1999 - 2012 Intel Corporation.
+
+ This program is free software; you can redistribute it and/or modify it
+ under the terms and conditions of the GNU General Public License,
+ version 2, as published by the Free Software Foundation.
+
+ This program is distributed in the hope it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ more details.
+
+ You should have received a copy of the GNU General Public License along with
+ this program; if not, write to the Free Software Foundation, Inc.,
+ 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+
+ The full GNU General Public License is included in this distribution in
+ the file called "COPYING".
+
+ Contact Information:
+ e1000-devel Mailing List <e1000-devel@lists.sourceforge.net>
+ Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
+
+*******************************************************************************/
+
+#ifdef CONFIG_DEBUG_FS
+
+#include <linux/debugfs.h>
+#include <linux/module.h>
+
+#include "ixgbe.h"
+
+static struct dentry *ixgbe_dbg_root;
+
+/**
+ * ixgbe_dbg_adapter_init - setup the debugfs directory for the adapter
+ * @adapter: the adapter that is starting up
+ **/
+void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter)
+{
+ const char *name = pci_name(adapter->pdev);
+
+ adapter->ixgbe_dbg_adapter = debugfs_create_dir(name, ixgbe_dbg_root);
+ if (!adapter->ixgbe_dbg_adapter)
+ e_dev_err("debugfs entry for %s failed\n", name);
+}
+
+/**
+ * ixgbe_dbg_adapter_exit - clear out the adapter's debugfs entries
+ * @pf: the pf that is stopping
+ **/
+void ixgbe_dbg_adapter_exit(struct ixgbe_adapter *adapter)
+{
+ if (adapter->ixgbe_dbg_adapter)
+ debugfs_remove_recursive(adapter->ixgbe_dbg_adapter);
+ adapter->ixgbe_dbg_adapter = NULL;
+}
+
+/**
+ * ixgbe_dbg_init - start up debugfs for the driver
+ **/
+void ixgbe_dbg_init(void)
+{
+ ixgbe_dbg_root = debugfs_create_dir(ixgbe_driver_name, NULL);
+ if (ixgbe_dbg_root == NULL)
+ pr_err("init of debugfs failed\n");
+}
+
+/**
+ * ixgbe_dbg_exit - clean out the driver's debugfs entries
+ **/
+void ixgbe_dbg_exit(void)
+{
+ debugfs_remove_recursive(ixgbe_dbg_root);
+}
+
+#endif /* CONFIG_DEBUG_FS */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index e641f14..b3b846b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7448,6 +7448,10 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
e_err(probe, "failed to allocate sysfs resources\n");
#endif /* CONFIG_IXGBE_HWMON */
+#ifdef CONFIG_DEBUG_FS
+ ixgbe_dbg_adapter_init(adapter);
+#endif /* CONFIG_DEBUG_FS */
+
return 0;
err_register:
@@ -7482,6 +7486,10 @@ static void __devexit ixgbe_remove(struct pci_dev *pdev)
struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
struct net_device *netdev = adapter->netdev;
+#ifdef CONFIG_DEBUG_FS
+ ixgbe_dbg_adapter_exit(adapter);
+#endif /*CONFIG_DEBUG_FS */
+
set_bit(__IXGBE_DOWN, &adapter->state);
cancel_work_sync(&adapter->service_task);
@@ -7737,6 +7745,10 @@ static int __init ixgbe_init_module(void)
pr_info("%s - version %s\n", ixgbe_driver_string, ixgbe_driver_version);
pr_info("%s\n", ixgbe_copyright);
+#ifdef CONFIG_DEBUG_FS
+ ixgbe_dbg_init();
+#endif /* CONFIG_DEBUG_FS */
+
#ifdef CONFIG_IXGBE_DCA
dca_register_notify(&dca_notifier);
#endif
@@ -7759,6 +7771,11 @@ static void __exit ixgbe_exit_module(void)
dca_unregister_notify(&dca_notifier);
#endif
pci_unregister_driver(&ixgbe_driver);
+
+#ifdef CONFIG_DEBUG_FS
+ ixgbe_dbg_exit();
+#endif /* CONFIG_DEBUG_FS */
+
rcu_barrier(); /* Wait for completion of call_rcu()'s */
}
--
1.7.11.4
^ permalink raw reply related
* [net-next 7/8] ixgbe: added reg_ops file to debugfs
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Catherine Sullivan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Catherine Sullivan <catherine.sullivan@intel.com>
Added the reg_ops file to debugfs with commands to read and write
a register to give users the ability to read and write individual
registers on the fly.
Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 118 +++++++++++++++++++++++
1 file changed, 118 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index 2dd169e..8d3a218 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -34,6 +34,119 @@
static struct dentry *ixgbe_dbg_root;
+static char ixgbe_dbg_reg_ops_buf[256] = "";
+
+/**
+ * ixgbe_dbg_reg_ops_open - prep the debugfs pokee data item when opened
+ * @inode: inode that was opened
+ * @filp: file info
+ *
+ * Stash the adapter pointer hiding in the inode into the file pointer where
+ * we can find it later in the read and write calls
+ **/
+static int ixgbe_dbg_reg_ops_open(struct inode *inode, struct file *filp)
+{
+ filp->private_data = inode->i_private;
+ return 0;
+}
+
+/**
+ * ixgbe_dbg_reg_ops_read - read for reg_ops datum
+ * @filp: the opened file
+ * @buffer: where to write the data for the user to read
+ * @count: the size of the user's buffer
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_reg_ops_read(struct file *filp, char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ixgbe_adapter *adapter = filp->private_data;
+ char buf[256];
+ int bytes_not_copied;
+ int len;
+
+ /* don't allow partial reads */
+ if (*ppos != 0)
+ return 0;
+
+ len = snprintf(buf, sizeof(buf), "%s: %s\n",
+ adapter->netdev->name, ixgbe_dbg_reg_ops_buf);
+ if (count < len)
+ return -ENOSPC;
+ bytes_not_copied = copy_to_user(buffer, buf, len);
+ if (bytes_not_copied < 0)
+ return bytes_not_copied;
+
+ *ppos = len;
+ return len;
+}
+
+/**
+ * ixgbe_dbg_reg_ops_write - write into reg_ops datum
+ * @filp: the opened file
+ * @buffer: where to find the user's data
+ * @count: the length of the user's data
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
+ const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ixgbe_adapter *adapter = filp->private_data;
+ int bytes_not_copied;
+
+ /* don't allow partial writes */
+ if (*ppos != 0)
+ return 0;
+ if (count >= sizeof(ixgbe_dbg_reg_ops_buf))
+ return -ENOSPC;
+
+ bytes_not_copied = copy_from_user(ixgbe_dbg_reg_ops_buf, buffer, count);
+ if (bytes_not_copied < 0)
+ return bytes_not_copied;
+ else if (bytes_not_copied < count)
+ count -= bytes_not_copied;
+ else
+ return -ENOSPC;
+ ixgbe_dbg_reg_ops_buf[count] = '\0';
+
+ if (strncmp(ixgbe_dbg_reg_ops_buf, "write", 5) == 0) {
+ u32 reg, value;
+ int cnt;
+ cnt = sscanf(&ixgbe_dbg_reg_ops_buf[5], "%x %x", ®, &value);
+ if (cnt == 2) {
+ IXGBE_WRITE_REG(&adapter->hw, reg, value);
+ value = IXGBE_READ_REG(&adapter->hw, reg);
+ e_dev_info("write: 0x%08x = 0x%08x\n", reg, value);
+ } else {
+ e_dev_info("write <reg> <value>\n");
+ }
+ } else if (strncmp(ixgbe_dbg_reg_ops_buf, "read", 4) == 0) {
+ u32 reg, value;
+ int cnt;
+ cnt = sscanf(&ixgbe_dbg_reg_ops_buf[4], "%x", ®);
+ if (cnt == 1) {
+ value = IXGBE_READ_REG(&adapter->hw, reg);
+ e_dev_info("read 0x%08x = 0x%08x\n", reg, value);
+ } else {
+ e_dev_info("read <reg>\n");
+ }
+ } else {
+ e_dev_info("Unknown command %s\n", ixgbe_dbg_reg_ops_buf);
+ e_dev_info("Available commands:\n");
+ e_dev_info(" read <reg>\n");
+ e_dev_info(" write <reg> <value>\n");
+ }
+ return count;
+}
+
+static const struct file_operations ixgbe_dbg_reg_ops_fops = {
+ .owner = THIS_MODULE,
+ .open = ixgbe_dbg_reg_ops_open,
+ .read = ixgbe_dbg_reg_ops_read,
+ .write = ixgbe_dbg_reg_ops_write,
+};
+
static char ixgbe_dbg_netdev_ops_buf[256] = "";
/**
@@ -140,6 +253,11 @@ void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter)
struct dentry *pfile;
adapter->ixgbe_dbg_adapter = debugfs_create_dir(name, ixgbe_dbg_root);
if (adapter->ixgbe_dbg_adapter) {
+ pfile = debugfs_create_file("reg_ops", 0600,
+ adapter->ixgbe_dbg_adapter, adapter,
+ &ixgbe_dbg_reg_ops_fops);
+ if (!pfile)
+ e_dev_err("debugfs reg_ops for %s failed\n", name);
pfile = debugfs_create_file("netdev_ops", 0600,
adapter->ixgbe_dbg_adapter, adapter,
&ixgbe_dbg_netdev_ops_fops);
--
1.7.11.4
^ permalink raw reply related
* [net-next 6/8] ixgbe: added netdev_ops file to debugfs
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Catherine Sullivan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Catherine Sullivan <catherine.sullivan@intel.com>
Added the netdev_ops file to debugfs with a command to call the
ndo_tx_timeout function to give users the ability to simulate a
tx_timeout call made by the kernel.
Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 107 ++++++++++++++++++++++-
1 file changed, 105 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index 0b08b6c..2dd169e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -34,6 +34,102 @@
static struct dentry *ixgbe_dbg_root;
+static char ixgbe_dbg_netdev_ops_buf[256] = "";
+
+/**
+ * ixgbe_dbg_netdev_ops_open - prep the debugfs netdev_ops data item
+ * @inode: inode that was opened
+ * @filp: file info
+ *
+ * Stash the adapter pointer hiding in the inode into the file pointer
+ * where we can find it later in the read and write calls
+ **/
+static int ixgbe_dbg_netdev_ops_open(struct inode *inode, struct file *filp)
+{
+ filp->private_data = inode->i_private;
+ return 0;
+}
+
+/**
+ * ixgbe_dbg_netdev_ops_read - read for netdev_ops datum
+ * @filp: the opened file
+ * @buffer: where to write the data for the user to read
+ * @count: the size of the user's buffer
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_netdev_ops_read(struct file *filp,
+ char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ixgbe_adapter *adapter = filp->private_data;
+ char buf[256];
+ int bytes_not_copied;
+ int len;
+
+ /* don't allow partial reads */
+ if (*ppos != 0)
+ return 0;
+
+ len = snprintf(buf, sizeof(buf), "%s: %s\n",
+ adapter->netdev->name, ixgbe_dbg_netdev_ops_buf);
+ if (count < len)
+ return -ENOSPC;
+ bytes_not_copied = copy_to_user(buffer, buf, len);
+ if (bytes_not_copied < 0)
+ return bytes_not_copied;
+
+ *ppos = len;
+ return len;
+}
+
+/**
+ * ixgbe_dbg_netdev_ops_write - write into netdev_ops datum
+ * @filp: the opened file
+ * @buffer: where to find the user's data
+ * @count: the length of the user's data
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_netdev_ops_write(struct file *filp,
+ const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ixgbe_adapter *adapter = filp->private_data;
+ int bytes_not_copied;
+
+ /* don't allow partial writes */
+ if (*ppos != 0)
+ return 0;
+ if (count >= sizeof(ixgbe_dbg_netdev_ops_buf))
+ return -ENOSPC;
+
+ bytes_not_copied = copy_from_user(ixgbe_dbg_netdev_ops_buf,
+ buffer, count);
+ if (bytes_not_copied < 0)
+ return bytes_not_copied;
+ else if (bytes_not_copied < count)
+ count -= bytes_not_copied;
+ else
+ return -ENOSPC;
+ ixgbe_dbg_netdev_ops_buf[count] = '\0';
+
+ if (strncmp(ixgbe_dbg_netdev_ops_buf, "tx_timeout", 10) == 0) {
+ adapter->netdev->netdev_ops->ndo_tx_timeout(adapter->netdev);
+ e_dev_info("tx_timeout called\n");
+ } else {
+ e_dev_info("Unknown command: %s\n", ixgbe_dbg_netdev_ops_buf);
+ e_dev_info("Available commands:\n");
+ e_dev_info(" tx_timeout\n");
+ }
+ return count;
+}
+
+static const struct file_operations ixgbe_dbg_netdev_ops_fops = {
+ .owner = THIS_MODULE,
+ .open = ixgbe_dbg_netdev_ops_open,
+ .read = ixgbe_dbg_netdev_ops_read,
+ .write = ixgbe_dbg_netdev_ops_write,
+};
+
/**
* ixgbe_dbg_adapter_init - setup the debugfs directory for the adapter
* @adapter: the adapter that is starting up
@@ -41,10 +137,17 @@ static struct dentry *ixgbe_dbg_root;
void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter)
{
const char *name = pci_name(adapter->pdev);
-
+ struct dentry *pfile;
adapter->ixgbe_dbg_adapter = debugfs_create_dir(name, ixgbe_dbg_root);
- if (!adapter->ixgbe_dbg_adapter)
+ if (adapter->ixgbe_dbg_adapter) {
+ pfile = debugfs_create_file("netdev_ops", 0600,
+ adapter->ixgbe_dbg_adapter, adapter,
+ &ixgbe_dbg_netdev_ops_fops);
+ if (!pfile)
+ e_dev_err("debugfs netdev_ops for %s failed\n", name);
+ } else {
e_dev_err("debugfs entry for %s failed\n", name);
+ }
}
/**
--
1.7.11.4
^ permalink raw reply related
* [net-next 8/8] ixgbe: Improve statistics accuracy for DDP traffic
From: Jeff Kirsher @ 2012-09-17 4:15 UTC (permalink / raw)
To: davem; +Cc: Mark Rustad, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1347855342-6610-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Mark Rustad <mark.d.rustad@intel.com>
Noticed that the byte and packet count statistics are under-
counting traffic handled by the DDP offload when there is more
than one DDP completion processed in a single call to
ixgbe_clean_rx_irq. This patch fixes that.
I tried to optimize the setting of the rss value so that it
only would have to be computed once, and only when there is
a DDP completion present.
Signed-off-by: Mark Rustad <mark.d.rustad@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Tested-by: Ross Brattain <ross.b.brattain@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 32 +++++++++++++--------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b3b846b..2dc9d91 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1785,7 +1785,8 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
unsigned int total_rx_bytes = 0, total_rx_packets = 0;
#ifdef IXGBE_FCOE
struct ixgbe_adapter *adapter = q_vector->adapter;
- int ddp_bytes = 0;
+ int ddp_bytes;
+ unsigned int mss = 0;
#endif /* IXGBE_FCOE */
u16 cleaned_count = ixgbe_desc_unused(rx_ring);
@@ -1839,6 +1840,20 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
/* if ddp, not passing to ULD unless for FCP_RSP or error */
if (ixgbe_rx_is_fcoe(rx_ring, rx_desc)) {
ddp_bytes = ixgbe_fcoe_ddp(adapter, rx_desc, skb);
+ /* include DDPed FCoE data */
+ if (ddp_bytes > 0) {
+ if (!mss) {
+ mss = rx_ring->netdev->mtu -
+ sizeof(struct fcoe_hdr) -
+ sizeof(struct fc_frame_header) -
+ sizeof(struct fcoe_crc_eof);
+ if (mss > 512)
+ mss &= ~511;
+ }
+ total_rx_bytes += ddp_bytes;
+ total_rx_packets += DIV_ROUND_UP(ddp_bytes,
+ mss);
+ }
if (!ddp_bytes) {
dev_kfree_skb_any(skb);
continue;
@@ -1852,21 +1867,6 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
budget--;
} while (likely(budget));
-#ifdef IXGBE_FCOE
- /* include DDPed FCoE data */
- if (ddp_bytes > 0) {
- unsigned int mss;
-
- mss = rx_ring->netdev->mtu - sizeof(struct fcoe_hdr) -
- sizeof(struct fc_frame_header) -
- sizeof(struct fcoe_crc_eof);
- if (mss > 512)
- mss &= ~511;
- total_rx_bytes += ddp_bytes;
- total_rx_packets += DIV_ROUND_UP(ddp_bytes, mss);
- }
-
-#endif /* IXGBE_FCOE */
u64_stats_update_begin(&rx_ring->syncp);
rx_ring->stats.packets += total_rx_packets;
rx_ring->stats.bytes += total_rx_bytes;
--
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