* [PATCH net-next-2.6 2/3] sfc: Limit filter search depth further for performance hints (i.e. RFS)
From: Ben Hutchings @ 2011-02-18 16:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1298044839.2570.7.camel@bwh-desktop>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/sfc/filter.c | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/sfc/filter.c b/drivers/net/sfc/filter.c
index d4722c4..47a1b79 100644
--- a/drivers/net/sfc/filter.c
+++ b/drivers/net/sfc/filter.c
@@ -27,6 +27,10 @@
*/
#define FILTER_CTL_SRCH_MAX 200
+/* Don't try very hard to find space for performance hints, as this is
+ * counter-productive. */
+#define FILTER_CTL_SRCH_HINT_MAX 5
+
enum efx_filter_table_id {
EFX_FILTER_TABLE_RX_IP = 0,
EFX_FILTER_TABLE_RX_MAC,
@@ -325,15 +329,16 @@ static int efx_filter_search(struct efx_filter_table *table,
struct efx_filter_spec *spec, u32 key,
bool for_insert, int *depth_required)
{
- unsigned hash, incr, filter_idx, depth;
+ unsigned hash, incr, filter_idx, depth, depth_max;
struct efx_filter_spec *cmp;
hash = efx_filter_hash(key);
incr = efx_filter_increment(key);
+ depth_max = (spec->priority <= EFX_FILTER_PRI_HINT ?
+ FILTER_CTL_SRCH_HINT_MAX : FILTER_CTL_SRCH_MAX);
for (depth = 1, filter_idx = hash & (table->size - 1);
- depth <= FILTER_CTL_SRCH_MAX &&
- test_bit(filter_idx, table->used_bitmap);
+ depth <= depth_max && test_bit(filter_idx, table->used_bitmap);
++depth) {
cmp = &table->spec[filter_idx];
if (efx_filter_equal(spec, cmp))
@@ -342,7 +347,7 @@ static int efx_filter_search(struct efx_filter_table *table,
}
if (!for_insert)
return -ENOENT;
- if (depth > FILTER_CTL_SRCH_MAX)
+ if (depth > depth_max)
return -EBUSY;
found:
*depth_required = depth;
--
1.7.3.4
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related
* [PATCH net-next-2.6 1/3] net: RPS: Make hardware-accelerated RFS conditional on NETIF_F_NTUPLE
From: Ben Hutchings @ 2011-02-18 16:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1298044839.2570.7.camel@bwh-desktop>
For testing and debugging purposes it is useful to be able to disable
hardware acceleration of RFS without disabling RFS altogether. Since
this is a similar feature to 'n-tuple' flow steering through the
ethtool API, test the same feature flag that controls that.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
net/core/dev.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 30c71f9..54aaca6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2607,7 +2607,8 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
int rc;
/* Should we steer this flow to a different hardware queue? */
- if (!skb_rx_queue_recorded(skb) || !dev->rx_cpu_rmap)
+ if (!skb_rx_queue_recorded(skb) || !dev->rx_cpu_rmap ||
+ !(dev->features & NETIF_F_NTUPLE))
goto out;
rxq_index = cpu_rmap_lookup_index(dev->rx_cpu_rmap, next_cpu);
if (rxq_index == skb_get_rx_queue(skb))
--
1.7.3.4
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related
* pull request: sfc-next-2.6 2011-02-18
From: Ben Hutchings @ 2011-02-18 16:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev, sf-linux-drivers, Tom Herbert
The following changes since commit f878b995b0f746f5726af9e66940f3bf373dae91:
Merge branch 'for-davem' of git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6 (2011-02-15 12:25:19 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git for-davem
Add support for RFS acceleration in the sfc driver, and allow it to be
enabled and disabled dynamically using ethtool.
Ben.
Ben Hutchings (3):
net: RPS: Make hardware-accelerated RFS conditional on NETIF_F_NTUPLE
sfc: Limit filter search depth further for performance hints (i.e. RFS)
sfc: Implement hardware acceleration of RFS
drivers/net/sfc/efx.c | 49 +++++++++++++++++-
drivers/net/sfc/efx.h | 15 +++++
drivers/net/sfc/filter.c | 117 ++++++++++++++++++++++++++++++++++++++++--
drivers/net/sfc/net_driver.h | 3 +
net/core/dev.c | 3 +-
5 files changed, 180 insertions(+), 7 deletions(-)
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 ethtool] ethtool: Warn if we cannot set the advertising mask for specified speed/duplex
From: Ben Hutchings @ 2011-02-18 15:56 UTC (permalink / raw)
To: netdev; +Cc: linux-net-drivers, Naveen Chouksey
In-Reply-To: <1297969576.2637.33.camel@bwh-desktop>
On Thu, 2011-02-17 at 19:06 +0000, Ben Hutchings wrote:
> When autonegotiation is enabled, drivers must determine link speed and
> duplex through the autonegotiation process and will generally ignore
> the speed and duplex specified in struct ethtool_cmd. If the user
> specifies a recognised combination of speed and duplex, we set the
> advertising mask to include only the matching mode. Otherwise, we
> advertise all supported modes. We should really limit the advertised
> modes separately by speed and duplex, but for now we just warn.
>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> This is a longstanding bug in ethtool which people keep getting bitten
> by. Naveen was just the latest to report it. I have been working on
> some changes to improve link advertising and reporting, but until those
> are ready I'm adding this warning.
That patch fixes an additional bug: we will now update the advertising
mask based on speed/duplex when autoneg is already on. Unfortunately it
introduces a bug: we will update advertising even if none of autoneg,
speed or duplex are specified!
Here's a second version which I think will do the right thing in all
cases, though I haven't tested it yet. I'll include this in ethtool
2.6.38 if no-one can spot a flaw.
Ben.
---
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 17 Feb 2011 18:51:15 +0000
Subject: [PATCH ethtool] ethtool: Don't silently ignore speed/duplex when autoneg is on
When autonegotiation is enabled, drivers must determine link speed and
duplex through the autonegotiation process and will generally ignore
the speed and duplex specified in struct ethtool_cmd. Currently, if
the user specifies autoneg on but does not specify the advertising
mask then:
- If the user specifies a recognised combination of speed and duplex,
we set the advertising mask to the flag for that mode. (Currently
only one mode is recognised per combination of speed and duplex.)
- Otherwise, we advertise all recognised and supported modes.
But we should also set the advertising mask if autoneg is *already*
on. Also, we should be able to limit the advertised modes separately
by speed and duplex. For now, we just warn if we fail to do that.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
ethtool.c | 45 +++++++++++++++++++++++++++++++--------------
1 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index 1afdfe4..2f4b6ee 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1126,7 +1126,7 @@ static void parse_cmdline(int argc, char **argp)
}
}
- if ((autoneg_wanted == AUTONEG_ENABLE) && (advertising_wanted < 0)) {
+ if (advertising_wanted < 0) {
if (speed_wanted == SPEED_10 && duplex_wanted == DUPLEX_HALF)
advertising_wanted = ADVERTISED_10baseT_Half;
else if (speed_wanted == SPEED_10 &&
@@ -2528,19 +2528,36 @@ static int do_sset(int fd, struct ifreq *ifr)
ecmd.phy_address = phyad_wanted;
if (xcvr_wanted != -1)
ecmd.transceiver = xcvr_wanted;
- if (advertising_wanted != -1) {
- if (advertising_wanted == 0)
- ecmd.advertising = ecmd.supported &
- (ADVERTISED_10baseT_Half |
- ADVERTISED_10baseT_Full |
- ADVERTISED_100baseT_Half |
- ADVERTISED_100baseT_Full |
- ADVERTISED_1000baseT_Half |
- ADVERTISED_1000baseT_Full |
- ADVERTISED_2500baseX_Full |
- ADVERTISED_10000baseT_Full);
- else
- ecmd.advertising = advertising_wanted;
+ /* XXX If the user specified speed or duplex
+ * then we should mask the advertised modes
+ * accordingly. For now, warn that we aren't
+ * doing that.
+ */
+ if ((speed_wanted != -1 || duplex_wanted != -1) &&
+ ecmd.autoneg && advertising_wanted == 0) {
+ fprintf(stderr, "Cannot advertise");
+ if (speed_wanted >= 0)
+ fprintf(stderr, " speed %d",
+ speed_wanted);
+ if (duplex_wanted >= 0)
+ fprintf(stderr, " duplex %s",
+ duplex_wanted ?
+ "full" : "half");
+ fprintf(stderr, "\n");
+ }
+ if (autoneg_wanted == AUTONEG_ENABLE &&
+ advertising_wanted == 0) {
+ ecmd.advertising = ecmd.supported &
+ (ADVERTISED_10baseT_Half |
+ ADVERTISED_10baseT_Full |
+ ADVERTISED_100baseT_Half |
+ ADVERTISED_100baseT_Full |
+ ADVERTISED_1000baseT_Half |
+ ADVERTISED_1000baseT_Full |
+ ADVERTISED_2500baseX_Full |
+ ADVERTISED_10000baseT_Full);
+ } else if (advertising_wanted != -1) {
+ ecmd.advertising = advertising_wanted;
}
/* Try to perform the update. */
--
1.7.3.4
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related
* Re: [patch net-next-2.6] net: convert bonding to use rx_handler
From: Patrick McHardy @ 2011-02-18 15:50 UTC (permalink / raw)
To: Jiri Pirko
Cc: Eric Dumazet, netdev, davem, shemminger, fubar, nicolas.2p.debian,
andy
In-Reply-To: <20110218145850.GF2939@psychotron.redhat.com>
On 18.02.2011 15:58, Jiri Pirko wrote:
> Fri, Feb 18, 2011 at 03:46:45PM CET, kaber@trash.net wrote:
>> Am 18.02.2011 15:27, schrieb Eric Dumazet:
>>> Le vendredi 18 février 2011 à 15:14 +0100, Jiri Pirko a écrit :
>>>
>>>> Do not know how to do it better. As for percpu variable, not only
>>>> origdev would have to be remembered but also probably skb pointer to
>>>> know if it's the first run on the skb or not. Can't really figure out a
>>>> better solution. Can you?
>>>
>>> I'll try and let you know.
>>
>> Why not simply do a lookup on skb->iif?
>
> Well I was trying to avoid iterating over list of devices for each
> incoming frame.
>
Well, there are a couple of holes on 64 bit, perhaps you can rearrange
things and eliminate either iif or input_dev without increasing size
since they appear to be redundant.
^ permalink raw reply
* Re: [PATCH v2 09/13] can: pruss CAN driver.
From: Arnd Bergmann @ 2011-02-18 15:07 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Subhasish Ghosh, davinci-linux-open-source, sachi, nsekhar,
open list, open list:CAN NETWORK DRIVERS,
open list:CAN NETWORK DRIVERS, m-watkins, Wolfgang Grandegger
In-Reply-To: <1297435892-28278-10-git-send-email-subhasish@mistralsolutions.com>
On Friday 11 February 2011, Subhasish Ghosh wrote:
> This patch adds support for the CAN device emulated on PRUSS.
Hi Subhasish,
This is a detailed walk through the can driver. The pruss_can.c
file mostly looks good, there are very tiny changes that I'm
suggesting to improve the code. I assume that you wrote that file.
The pruss_can_api.c is a bit of a mess and looks like it was copied
from some other code base and just barely changed to follow Linux
coding style. I can tell from the main driver file that you can do
better than that.
My recommendation for that would be to throw it away and reimplement
the few parts that you actually need, in proper coding style.
You can also try to fix the file according to the comments I give
you below, but I assume that would be signficantly more work.
Moving everything into one file also makes things easier to read
here and lets you identifer more quickly what is unused.
Arnd
> +#ifdef __CAN_DEBUG
> +#define __can_debug(fmt, args...) printk(KERN_DEBUG "can_debug: " fmt, ## args)
> +#else
> +#define __can_debug(fmt, args...)
> +#endif
> +#define __can_err(fmt, args...) printk(KERN_ERR "can_err: " fmt, ## args)
Better use the existing dev_dbg() and dev_err() macros that provide the
same functionality in a more standard way. You already use them in
some places, as I noticed.
If you don't have a way to pass a meaningful device, you can use
pr_debug/pr_err.
> +void omapl_pru_can_rx_wQ(struct work_struct *work)
> +{
This is only used in the same file, so better make it static.
> + if (-1 == pru_can_get_intr_status(priv->dev, &priv->can_rx_hndl))
> + return;
Don't make up your own return values, just use the standard error codes,
e.g. -EIO or -EAGAIN, whatever fits.
The more common way to write the comparison would be the other way round,
if (pru_can_get_intr_status(priv->dev, &priv->can_rx_hndl) == -EAGAIN)
return;
or, simpler
if (pru_can_get_intr_status(priv->dev, &priv->can_rx_hndl))
return;
> +irqreturn_t omapl_tx_can_intr(int irq, void *dev_id)
> +{
This also should be static
> + for (bit_set = 0; ((priv->can_tx_hndl.u32interruptstatus & 0xFF)
> + >> bit_set != 0); bit_set++)
> + ;
bit_set = fls(priv->can_tx_hndl.u32interruptstatus & 0xFF); ?
> +irqreturn_t omapl_rx_can_intr(int irq, void *dev_id)
static
> diff --git a/drivers/net/can/da8xx_pruss/pruss_can_api.c b/drivers/net/can/da8xx_pruss/pruss_can_api.c
> new file mode 100644
> index 0000000..2f7438a
> --- /dev/null
> +++ b/drivers/net/can/da8xx_pruss/pruss_can_api.c
A lot of code in this file seems to be unused. Is that right?
I would suggest adding only the code that is actually being
used. If you add more functionality later, you can always
add back the low-level functions, but dead code usually
turns into broken code quickly.
> +static can_emu_drv_inst gstr_can_inst[ecanmaxinst];
This is global data and probably needs some for of locking
> +/*
> + * pru_can_set_brp() Updates the BRP register of PRU0
> + * and PRU1 of OMAP L138. This API will be called by the
> + * Application to updtae the BRP register of PRU0 and PRU1
> + *
> + * param u16bitrateprescaler The can bus bitrate
> + * prescaler value be set
> + *
> + * return SUCCESS or FAILURE
> + */
> +s16 pru_can_set_brp(struct device *dev, u16 u16bitrateprescaler)
> +{
unused.
> + u32 u32offset;
> +
> + if (u16bitrateprescaler > 255) {
> + return -1;
> + }
non-standard error code. It also doesn't match the comment, which
claims it is SUCCESS or FAILURE, both of which are (rightfully)
not defined.
> + u32offset = (PRU_CAN_RX_CLOCK_BRP_REGISTER);
> + pruss_writel(dev, u32offset, (u32 *) &u16bitrateprescaler, 1);
> +
> + u32offset = (PRU_CAN_TX_CLOCK_BRP_REGISTER);
> + pruss_writel(dev, u32offset, (u32 *) &u16bitrateprescaler, 1);
You pass a 32 bit pointer to a 16 bit local variable here.
This has an undefined effect, and if you build this code on
a big-endian platform, it cannot possibly do anything good.
pruss_writel() is defined in a funny way if it takes a thirty-two bit
input argument by reference, rather than by value. What is going
on there?
> +s16 pru_can_set_bit_timing(struct device *dev,
> + can_bit_timing_consts *pstrbittiming)
unused.
> + u32 u32offset;
> + u32 u32serregister;
It's a bit silly to put the name of the type into the name
of a variable. You already spell it out in the definition.
> +s16 pru_can_write_data_to_mailbox(struct device *dev,
> + can_emu_app_hndl *pstremuapphndl)
> +{
> + s16 s16subrtnretval;
> + u32 u32offset;
> +
> + if (pstremuapphndl == NULL) {
> + return -1;
> + }
nonstandard error code. Also, why the heck is type function
return type s16 when the only possible return values are 0
and -1? Just make this an int.
> + switch ((u8) pstremuapphndl->ecanmailboxnumber) {
> + case 0:
> + u32offset = (PRU_CAN_TX_MAILBOX0);
> + break;
> + case 1:
> + u32offset = (PRU_CAN_TX_MAILBOX1);
> + break;
> + case 2:
> + u32offset = (PRU_CAN_TX_MAILBOX2);
> + break;
> + case 3:
> + u32offset = (PRU_CAN_TX_MAILBOX3);
> + break;
> + case 4:
> + u32offset = (PRU_CAN_TX_MAILBOX4);
> + break;
> + case 5:
> + u32offset = (PRU_CAN_TX_MAILBOX5);
> + break;
> + case 6:
> + u32offset = (PRU_CAN_TX_MAILBOX6);
> + break;
> + case 7:
> + u32offset = (PRU_CAN_TX_MAILBOX7);
> + break;
> + default:
> + return -1;
> + }
Lovely switch statement. I'm sure you find a better way to express this ;-)
> + s16subrtnretval = pruss_writel(dev, u32offset,
> + (u32 *) &(pstremuapphndl->strcanmailbox), 4);
> + if (s16subrtnretval == -1) {
> + return -1;
> + }
> + return 0;
> +}
return pruss_writel(...) ?
> +
> +/*
> + * pru_can_get_intr_status()
> + * Gets the interrupts status register value.
> + * This API will be called by the Application
> + * to get the interrupts status register value
> + *
> + * param u8prunumber PRU number for which IntStatusReg
> + * has to be read
> + *
> + * return SUCCESS or FAILURE
> + */
> +s16 pru_can_get_intr_status(struct device *dev,
> + can_emu_app_hndl *pstremuapphndl)
> +{
> + u32 u32offset;
> + s16 s16subrtnretval = -1;
> +
> + if (pstremuapphndl == NULL) {
> + return -1;
> + }
In every function you check that pstremuapphndl is present. This seems
rather pointless. How about just making sure you never pass a NULL
value to these functions? That should not be hard at all from the
high-level driver.
> + if (pstremuapphndl->u8prunumber == DA8XX_PRUCORE_1) {
> + u32offset = (PRU_CAN_TX_INTERRUPT_STATUS_REGISTER);
> + } else if (pstremuapphndl->u8prunumber == DA8XX_PRUCORE_0) {
> + u32offset = (PRU_CAN_RX_INTERRUPT_STATUS_REGISTER);
> + } else {
> + return -1;
> + }
> +
> + s16subrtnretval = pruss_readl(dev, u32offset,
> + (u32 *) &pstremuapphndl->u32interruptstatus, 1);
> + if (s16subrtnretval == -1) {
> + return -1;
> + }
You can also get rid of all these {} braces around one-line statements.
> +/*
> + * pru_can_get_mailbox_status() Gets the mailbox status
> + * register value. This API will be called by the Application
> + * to get the mailbox status register value
> + *
> + * return SUCCESS or FAILURE
> + */
> +s16 pru_can_get_mailbox_status(struct device *dev,
> + can_emu_app_hndl *pstremuapphndl)
unused.
> +/*
> + * pru_can_config_mode_set() Sets the timing value
> + * for data transfer. This API will be called by the Application
> + * to set timing valus for data transfer
> + *
> + * return SUCCESS or FAILURE
> + */
> +s16 pru_can_config_mode_set(struct device *dev, bool bconfigmodeflag)
unused.
> + u32offset = (PRU_CAN_TX_GLOBAL_CONTROL_REGISTER & 0xFFFF);
> + u32value = 0x00000000;
> + s16subrtnretval = pruss_writel(dev, u32offset, (u32 *) &u32value, 1);
> + if (s16subrtnretval == -1) {
> + return -1;
> + }
> +
> + u32offset = (PRU_CAN_TX_GLOBAL_STATUS_REGISTER & 0xFFFF);
> + u32value = 0x00000040;
> + s16subrtnretval = pruss_writel(dev, u32offset, (u32 *) &u32value, 1);
> + if (s16subrtnretval == -1) {
> + return -1;
> + }
> + u32offset = (PRU_CAN_RX_GLOBAL_STATUS_REGISTER & 0xFFFF);
> + u32value = 0x00000040;
> + s16subrtnretval = pruss_writel(dev, u32offset, (u32 *) &u32value, 1);
> + if (s16subrtnretval == -1) {
> + return -1;
> + }
<skipping 50 (!) more of these>
After the third time of writing the same code, you should have noticed that
there is some duplication involved that can trivially be reduced. A good
way to express the same would be a table with the contents:
static struct pru_can_register_init {
u16 offset;
u32 value;
} = {
{ PRU_CAN_TX_GLOBAL_CONTROL_REGISTER, 0, },
{ PRU_CAN_TX_GLOBAL_STATUS_REGISTER, 0x40, },
{ PRU_CAN_RX_GLOBAL_STATUS_REGISTER, 0x40, },
...
};
> +
> +
> +/*
> + * pru_can_emu_open() Opens the can emu for
> + * application to use. This API will be called by the Application
> + * to Open the can emu for application to use.
> + *
> + * param pstremuapphndl Pointer to application handler
> + * structure
> + *
> + * return SUCCESS or FAILURE
> + */
> +s16 pru_can_emu_open(struct device *dev, can_emu_app_hndl *pstremuapphndl)
unused.
> +/*
> + * brief pru_can_emu_close() Closes the can emu for other
> + * applications to use. This API will be called by the Application to Close
> + * the can emu for other applications to use
> + *
> + * param pstremuapphndl Pointer to application handler structure
> + *
> + * return SUCCESS or FAILURE
> + */
> +s16 pru_can_emu_close(struct device *dev, can_emu_app_hndl *pstremuapphndl)
unused
> +s16 pru_can_emu_sreset(struct device *dev)
> +{
> + return 0;
> +}
pointless.
> +s16 pru_can_tx(struct device *dev, u8 u8mailboxnumber, u8 u8prunumber)
> +{
> + u32 u32offset = 0;
> + u32 u32value = 0;
> + s16 s16subrtnretval = -1;
> +
> + if (DA8XX_PRUCORE_1 == u8prunumber) {
> + switch (u8mailboxnumber) {
> + case 0:
> + u32offset = (PRU_CAN_TX_MAILBOX0_STATUS_REGISTER & 0xFFFF);
> + u32value = 0x00000080;
> + s16subrtnretval = pruss_writel(dev, u32offset,
> + (u32 *) &u32value, 1);
> + if (s16subrtnretval == -1) {
> + return -1;
> + }
> + break;
> + case 1:
> + u32offset = (PRU_CAN_TX_MAILBOX1_STATUS_REGISTER & 0xFFFF);
> + u32value = 0x00000080;
> + s16subrtnretval = pruss_writel(dev, u32offset,
> + (u32 *) &u32value, 1);
> + if (s16subrtnretval == -1) {
> + return -1;
> + }
> + break;
> + case 2:
> + u32offset = (PRU_CAN_TX_MAILBOX2_STATUS_REGISTER & 0xFFFF);
Another pointless switch statement.
> +s16 pru_can_mask_ints(struct device *dev, u32 int_mask)
> +{
> + return 0;
> +}
> +
> +int pru_can_get_error_cnt(struct device *dev, u8 u8prunumber)
> +{
> + return 0;
> +}
useless.
> +int pru_can_get_intc_status(struct device *dev)
> +{
> + u32 u32offset = 0;
> + u32 u32getvalue = 0;
> + u32 u32clrvalue = 0;
> +
> + u32offset = (PRUSS_INTC_STATCLRINT1 & 0xFFFF);
> + pruss_readl(dev, u32offset, (u32 *) &u32getvalue, 1);
> +
> + if (u32getvalue & 4)
> + u32clrvalue = 34; /* CLR Event 34 */
> +
> + if (u32getvalue & 2)
> + u32clrvalue = 33; /* CLR Event 33 */
> +
> + if (u32clrvalue) {
> + u32offset = (PRUSS_INTC_STATIDXCLR & 0xFFFF);
> + pruss_writel(dev, u32offset, &u32clrvalue, 1);
> + } else
> + return -1;
Could the controller signal both event 34 and 33 simultaneously?
The only user of this function looks at the individual bits
of the return value again, which looks wrong for all possible
return values here.
> +#ifndef _PRU_CAN_API_H_
> +#define CAN_BIT_TIMINGS (0x273)
> +
> +/* Timer Clock is sourced from DDR freq (PLL1 SYS CLK 2) */
> +#define TIMER_CLK_FREQ 132000000
> +
> +#define TIMER_SETUP_DELAY 14
> +#define GPIO_SETUP_DELAY 150
> +
> +#define CAN_RX_PRU_0 PRUSS_NUM0
> +#define CAN_TX_PRU_1 PRUSS_NUM1
> +
> +/* Number of Instruction in the Delay loop */
> +#define DELAY_LOOP_LENGTH 2
> +
> +#define PRU1_BASE_ADDR 0x2000
> +
> +#define PRU_CAN_TX_GLOBAL_CONTROL_REGISTER (PRU1_BASE_ADDR)
> +#define PRU_CAN_TX_GLOBAL_STATUS_REGISTER (PRU1_BASE_ADDR + 0x04)
> +#define PRU_CAN_TX_INTERRUPT_MASK_REGISTER (PRU1_BASE_ADDR + 0x08)
> +#define PRU_CAN_TX_INTERRUPT_STATUS_REGISTER (PRU1_BASE_ADDR + 0x0C)
> +#define PRU_CAN_TX_MAILBOX0_STATUS_REGISTER (PRU1_BASE_ADDR + 0x10)
> +#define PRU_CAN_TX_MAILBOX1_STATUS_REGISTER (PRU1_BASE_ADDR + 0x14)
The header file should be used for interfaces between the two .c files,
don't mix that with hardware specific definitions. Sometimes you may want
to have register number lists in a header, if that list is going to be
used in multiple places. In this case, there is just one user, so better
move all those definitions over there.
> +typedef enum {
> + ecaninst0 = 0,
> + ecaninst1,
> + ecanmaxinst
> +} can_instance_enum;
> +
> +typedef enum {
> + ecanmailbox0 = 0,
> + ecanmailbox1,
> + ecanmailbox2,
> + ecanmailbox3,
> + ecanmailbox4,
> + ecanmailbox5,
> + ecanmailbox6,
> + ecanmailbox7
> +} can_mailbox_number;
> +
> +typedef enum {
> + ecandirectioninit = 0,
> + ecantransmit,
> + ecanreceive
> +} can_transfer_direction;
The values are all unused, you only use the typedefs.
IMHO it would be more sensible to just pass these as unsigned int
or u32 values, but if you prefer, there is no reason to just do
typedef u32 can_mailbox_number;
etc.
> +typedef struct {
> + u16 u16extendedidentifier;
> + u16 u16baseidentifier;
> + u8 u8data7;
> + u8 u8data6;
> + u8 u8data5;
> + u8 u8data4;
> + u8 u8data3;
> + u8 u8data2;
> + u8 u8data1;
> + u8 u8data0;
> + u16 u16datalength;
> + u16 u16crc;
> +} can_mail_box_structure;
Please don't use typedef for complex data structures, and learn about
better naming of identifiers. I would suggest writing this as
struct pru_can_mailbox {
u16 ext_id;
u16 base_id;
u8 data[8]; /* note: reverse order */
u16 len;
u16 crc;
};
Sames rules apply to the other structures.
Arnd
^ permalink raw reply
* Re: [patch net-next-2.6] net: convert bonding to use rx_handler
From: Jiri Pirko @ 2011-02-18 14:58 UTC (permalink / raw)
To: Patrick McHardy
Cc: Eric Dumazet, netdev, davem, shemminger, fubar, nicolas.2p.debian,
andy
In-Reply-To: <4D5E8655.5070304@trash.net>
Fri, Feb 18, 2011 at 03:46:45PM CET, kaber@trash.net wrote:
>Am 18.02.2011 15:27, schrieb Eric Dumazet:
>> Le vendredi 18 février 2011 à 15:14 +0100, Jiri Pirko a écrit :
>>
>>> Do not know how to do it better. As for percpu variable, not only
>>> origdev would have to be remembered but also probably skb pointer to
>>> know if it's the first run on the skb or not. Can't really figure out a
>>> better solution. Can you?
>>
>> I'll try and let you know.
>
>Why not simply do a lookup on skb->iif?
Well I was trying to avoid iterating over list of devices for each
incoming frame.
^ permalink raw reply
* Re: [patch net-next-2.6] net: convert bonding to use rx_handler
From: Patrick McHardy @ 2011-02-18 14:46 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jiri Pirko, netdev, davem, shemminger, fubar, nicolas.2p.debian,
andy
In-Reply-To: <1298039252.6201.66.camel@edumazet-laptop>
Am 18.02.2011 15:27, schrieb Eric Dumazet:
> Le vendredi 18 février 2011 à 15:14 +0100, Jiri Pirko a écrit :
>
>> Do not know how to do it better. As for percpu variable, not only
>> origdev would have to be remembered but also probably skb pointer to
>> know if it's the first run on the skb or not. Can't really figure out a
>> better solution. Can you?
>
> I'll try and let you know.
Why not simply do a lookup on skb->iif?
^ permalink raw reply
* Re: [PATCH v6 0/9] net: Unified offload configuration
From: Ben Hutchings @ 2011-02-18 14:29 UTC (permalink / raw)
To: Michał Mirosław; +Cc: David Miller, netdev
In-Reply-To: <20110218142238.GA18099@rere.qmqm.pl>
On Fri, 2011-02-18 at 15:22 +0100, Michał Mirosław wrote:
> On Thu, Feb 17, 2011 at 02:56:11PM -0800, David Miller wrote:
[...]
> > Please get rid of that annoying message spit out by netif_features_change(),
> > it's just spam. If we want notifications for stuff like this, use a
> > non-unicast netlink message so those who want to hear it can do so.
>
> You mean netdev_update_features() "Features changed" message? Is it ok
> to just demote it to DEBUG level or you want to remove it altogether?
> What about netdev_fix_features() messages?
I think you need to emit these messages at 'error' severity when fixing
up features for a newly-added device, but at 'debug' later on.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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-2.6] net: convert bonding to use rx_handler
From: Eric Dumazet @ 2011-02-18 14:27 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, shemminger, kaber, fubar, nicolas.2p.debian, andy
In-Reply-To: <20110218141456.GD2939@psychotron.redhat.com>
Le vendredi 18 février 2011 à 15:14 +0100, Jiri Pirko a écrit :
> Do not know how to do it better. As for percpu variable, not only
> origdev would have to be remembered but also probably skb pointer to
> know if it's the first run on the skb or not. Can't really figure out a
> better solution. Can you?
I'll try and let you know.
Thanks
^ permalink raw reply
* [patch 1/1] [PATCH] qeth: remove needless IPA-commands in offline
From: frank.blaschka @ 2011-02-18 14:22 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, Ursula Braun
In-Reply-To: <20110218142258.791988211@de.ibm.com>
[-- Attachment #1: 602-needless-ipa-commands.diff --]
[-- Type: text/plain, Size: 12373 bytes --]
From: Ursula Braun <ursula.braun@de.ibm.com>
If a qeth device is set offline, data and control subchannels are
cleared, which means removal of all IP Assist Primitive settings
implicitly. There is no need to delete those settings explicitly.
This patch removes all IP Assist invocations from offline.
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
---
drivers/s390/net/qeth_core.h | 1
drivers/s390/net/qeth_core_main.c | 52 ++++++++++-------------------------
drivers/s390/net/qeth_l2_main.c | 45 +++++++++----------------------
drivers/s390/net/qeth_l3_main.c | 55 ++------------------------------------
4 files changed, 33 insertions(+), 120 deletions(-)
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -741,7 +741,6 @@ struct qeth_card {
/* QDIO buffer handling */
struct qeth_qdio_info qdio;
struct qeth_perf_stats perf_stats;
- int use_hard_stop;
int read_or_write_problem;
struct qeth_osn_info osn_info;
struct qeth_discipline discipline;
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -302,12 +302,15 @@ static void qeth_issue_ipa_msg(struct qe
int com = cmd->hdr.command;
ipa_name = qeth_get_ipa_cmd_name(com);
if (rc)
- QETH_DBF_MESSAGE(2, "IPA: %s(x%X) for %s returned x%X \"%s\"\n",
- ipa_name, com, QETH_CARD_IFNAME(card),
- rc, qeth_get_ipa_msg(rc));
+ QETH_DBF_MESSAGE(2, "IPA: %s(x%X) for %s/%s returned "
+ "x%X \"%s\"\n",
+ ipa_name, com, dev_name(&card->gdev->dev),
+ QETH_CARD_IFNAME(card), rc,
+ qeth_get_ipa_msg(rc));
else
- QETH_DBF_MESSAGE(5, "IPA: %s(x%X) for %s succeeded\n",
- ipa_name, com, QETH_CARD_IFNAME(card));
+ QETH_DBF_MESSAGE(5, "IPA: %s(x%X) for %s/%s succeeded\n",
+ ipa_name, com, dev_name(&card->gdev->dev),
+ QETH_CARD_IFNAME(card));
}
static struct qeth_ipa_cmd *qeth_check_ipa_data(struct qeth_card *card,
@@ -1083,7 +1086,6 @@ static int qeth_setup_card(struct qeth_c
card->data.state = CH_STATE_DOWN;
card->state = CARD_STATE_DOWN;
card->lan_online = 0;
- card->use_hard_stop = 0;
card->read_or_write_problem = 0;
card->dev = NULL;
spin_lock_init(&card->vlanlock);
@@ -1732,20 +1734,22 @@ int qeth_send_control_data(struct qeth_c
};
}
+ if (reply->rc == -EIO)
+ goto error;
rc = reply->rc;
qeth_put_reply(reply);
return rc;
time_err:
+ reply->rc = -ETIME;
spin_lock_irqsave(&reply->card->lock, flags);
list_del_init(&reply->list);
spin_unlock_irqrestore(&reply->card->lock, flags);
- reply->rc = -ETIME;
atomic_inc(&reply->received);
+error:
atomic_set(&card->write.irq_pending, 0);
qeth_release_buffer(iob->channel, iob);
card->write.buf_no = (card->write.buf_no + 1) % QETH_CMD_BUFFER_NO;
- wake_up(&reply->wait_q);
rc = reply->rc;
qeth_put_reply(reply);
return rc;
@@ -2490,45 +2494,19 @@ int qeth_send_ipa_cmd(struct qeth_card *
}
EXPORT_SYMBOL_GPL(qeth_send_ipa_cmd);
-static int qeth_send_startstoplan(struct qeth_card *card,
- enum qeth_ipa_cmds ipacmd, enum qeth_prot_versions prot)
-{
- int rc;
- struct qeth_cmd_buffer *iob;
-
- iob = qeth_get_ipacmd_buffer(card, ipacmd, prot);
- rc = qeth_send_ipa_cmd(card, iob, NULL, NULL);
-
- return rc;
-}
-
int qeth_send_startlan(struct qeth_card *card)
{
int rc;
+ struct qeth_cmd_buffer *iob;
QETH_DBF_TEXT(SETUP, 2, "strtlan");
- rc = qeth_send_startstoplan(card, IPA_CMD_STARTLAN, 0);
+ iob = qeth_get_ipacmd_buffer(card, IPA_CMD_STARTLAN, 0);
+ rc = qeth_send_ipa_cmd(card, iob, NULL, NULL);
return rc;
}
EXPORT_SYMBOL_GPL(qeth_send_startlan);
-int qeth_send_stoplan(struct qeth_card *card)
-{
- int rc = 0;
-
- /*
- * TODO: according to the IPA format document page 14,
- * TCP/IP (we!) never issue a STOPLAN
- * is this right ?!?
- */
- QETH_DBF_TEXT(SETUP, 2, "stoplan");
-
- rc = qeth_send_startstoplan(card, IPA_CMD_STOPLAN, 0);
- return rc;
-}
-EXPORT_SYMBOL_GPL(qeth_send_stoplan);
-
int qeth_default_setadapterparms_cb(struct qeth_card *card,
struct qeth_reply *reply, unsigned long data)
{
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -202,17 +202,19 @@ static void qeth_l2_add_mc(struct qeth_c
kfree(mc);
}
-static void qeth_l2_del_all_mc(struct qeth_card *card)
+static void qeth_l2_del_all_mc(struct qeth_card *card, int del)
{
struct qeth_mc_mac *mc, *tmp;
spin_lock_bh(&card->mclock);
list_for_each_entry_safe(mc, tmp, &card->mc_list, list) {
- if (mc->is_vmac)
- qeth_l2_send_setdelmac(card, mc->mc_addr,
+ if (del) {
+ if (mc->is_vmac)
+ qeth_l2_send_setdelmac(card, mc->mc_addr,
IPA_CMD_DELVMAC, NULL);
- else
- qeth_l2_send_delgroupmac(card, mc->mc_addr);
+ else
+ qeth_l2_send_delgroupmac(card, mc->mc_addr);
+ }
list_del(&mc->list);
kfree(mc);
}
@@ -288,18 +290,13 @@ static int qeth_l2_send_setdelvlan(struc
qeth_l2_send_setdelvlan_cb, NULL);
}
-static void qeth_l2_process_vlans(struct qeth_card *card, int clear)
+static void qeth_l2_process_vlans(struct qeth_card *card)
{
struct qeth_vlan_vid *id;
QETH_CARD_TEXT(card, 3, "L2prcvln");
spin_lock_bh(&card->vlanlock);
list_for_each_entry(id, &card->vid_list, list) {
- if (clear)
- qeth_l2_send_setdelvlan(card, id->vid,
- IPA_CMD_DELVLAN);
- else
- qeth_l2_send_setdelvlan(card, id->vid,
- IPA_CMD_SETVLAN);
+ qeth_l2_send_setdelvlan(card, id->vid, IPA_CMD_SETVLAN);
}
spin_unlock_bh(&card->vlanlock);
}
@@ -379,19 +376,11 @@ static int qeth_l2_stop_card(struct qeth
dev_close(card->dev);
rtnl_unlock();
}
- if (!card->use_hard_stop ||
- recovery_mode) {
- __u8 *mac = &card->dev->dev_addr[0];
- rc = qeth_l2_send_delmac(card, mac);
- QETH_DBF_TEXT_(SETUP, 2, "Lerr%d", rc);
- }
+ card->info.mac_bits &= ~QETH_LAYER2_MAC_REGISTERED;
card->state = CARD_STATE_SOFTSETUP;
}
if (card->state == CARD_STATE_SOFTSETUP) {
- qeth_l2_process_vlans(card, 1);
- if (!card->use_hard_stop ||
- recovery_mode)
- qeth_l2_del_all_mc(card);
+ qeth_l2_del_all_mc(card, 0);
qeth_clear_ipacmd_list(card);
card->state = CARD_STATE_HARDSETUP;
}
@@ -405,7 +394,6 @@ static int qeth_l2_stop_card(struct qeth
qeth_clear_cmd_buffers(&card->read);
qeth_clear_cmd_buffers(&card->write);
}
- card->use_hard_stop = 0;
return rc;
}
@@ -705,7 +693,7 @@ static void qeth_l2_set_multicast_list(s
if (qeth_threads_running(card, QETH_RECOVER_THREAD) &&
(card->state != CARD_STATE_UP))
return;
- qeth_l2_del_all_mc(card);
+ qeth_l2_del_all_mc(card, 1);
spin_lock_bh(&card->mclock);
netdev_for_each_mc_addr(ha, dev)
qeth_l2_add_mc(card, ha->addr, 0);
@@ -907,10 +895,8 @@ static void qeth_l2_remove_device(struct
qeth_set_allowed_threads(card, 0, 1);
wait_event(card->wait_q, qeth_threads_running(card, 0xffffffff) == 0);
- if (cgdev->state == CCWGROUP_ONLINE) {
- card->use_hard_stop = 1;
+ if (cgdev->state == CCWGROUP_ONLINE)
qeth_l2_set_offline(cgdev);
- }
if (card->dev) {
unregister_netdev(card->dev);
@@ -1040,7 +1026,7 @@ contin:
if (card->info.type != QETH_CARD_TYPE_OSN &&
card->info.type != QETH_CARD_TYPE_OSM)
- qeth_l2_process_vlans(card, 0);
+ qeth_l2_process_vlans(card);
netif_tx_disable(card->dev);
@@ -1076,7 +1062,6 @@ contin:
return 0;
out_remove:
- card->use_hard_stop = 1;
qeth_l2_stop_card(card, 0);
ccw_device_set_offline(CARD_DDEV(card));
ccw_device_set_offline(CARD_WDEV(card));
@@ -1144,7 +1129,6 @@ static int qeth_l2_recover(void *ptr)
QETH_CARD_TEXT(card, 2, "recover2");
dev_warn(&card->gdev->dev,
"A recovery process has been started for the device\n");
- card->use_hard_stop = 1;
__qeth_l2_set_offline(card->gdev, 1);
rc = __qeth_l2_set_online(card->gdev, 1);
if (!rc)
@@ -1191,7 +1175,6 @@ static int qeth_l2_pm_suspend(struct ccw
if (gdev->state == CCWGROUP_OFFLINE)
return 0;
if (card->state == CARD_STATE_UP) {
- card->use_hard_stop = 1;
__qeth_l2_set_offline(card->gdev, 1);
} else
__qeth_l2_set_offline(card->gdev, 0);
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -510,8 +510,7 @@ static void qeth_l3_set_ip_addr_list(str
kfree(tbd_list);
}
-static void qeth_l3_clear_ip_list(struct qeth_card *card, int clean,
- int recover)
+static void qeth_l3_clear_ip_list(struct qeth_card *card, int recover)
{
struct qeth_ipaddr *addr, *tmp;
unsigned long flags;
@@ -530,11 +529,6 @@ static void qeth_l3_clear_ip_list(struct
addr = list_entry(card->ip_list.next,
struct qeth_ipaddr, entry);
list_del_init(&addr->entry);
- if (clean) {
- spin_unlock_irqrestore(&card->ip_lock, flags);
- qeth_l3_deregister_addr_entry(card, addr);
- spin_lock_irqsave(&card->ip_lock, flags);
- }
if (!recover || addr->is_multicast) {
kfree(addr);
continue;
@@ -1611,29 +1605,6 @@ static int qeth_l3_start_ipassists(struc
return 0;
}
-static int qeth_l3_put_unique_id(struct qeth_card *card)
-{
-
- int rc = 0;
- struct qeth_cmd_buffer *iob;
- struct qeth_ipa_cmd *cmd;
-
- QETH_CARD_TEXT(card, 2, "puniqeid");
-
- if ((card->info.unique_id & UNIQUE_ID_NOT_BY_CARD) ==
- UNIQUE_ID_NOT_BY_CARD)
- return -1;
- iob = qeth_get_ipacmd_buffer(card, IPA_CMD_DESTROY_ADDR,
- QETH_PROT_IPV6);
- cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
- *((__u16 *) &cmd->data.create_destroy_addr.unique_id[6]) =
- card->info.unique_id;
- memcpy(&cmd->data.create_destroy_addr.unique_id[0],
- card->dev->dev_addr, OSA_ADDR_LEN);
- rc = qeth_send_ipa_cmd(card, iob, NULL, NULL);
- return rc;
-}
-
static int qeth_l3_iqd_read_initial_mac_cb(struct qeth_card *card,
struct qeth_reply *reply, unsigned long data)
{
@@ -2324,25 +2295,14 @@ static int qeth_l3_stop_card(struct qeth
dev_close(card->dev);
rtnl_unlock();
}
- if (!card->use_hard_stop) {
- rc = qeth_send_stoplan(card);
- if (rc)
- QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
- }
card->state = CARD_STATE_SOFTSETUP;
}
if (card->state == CARD_STATE_SOFTSETUP) {
- qeth_l3_clear_ip_list(card, !card->use_hard_stop, 1);
+ qeth_l3_clear_ip_list(card, 1);
qeth_clear_ipacmd_list(card);
card->state = CARD_STATE_HARDSETUP;
}
if (card->state == CARD_STATE_HARDSETUP) {
- if (!card->use_hard_stop &&
- (card->info.type != QETH_CARD_TYPE_IQD)) {
- rc = qeth_l3_put_unique_id(card);
- if (rc)
- QETH_DBF_TEXT_(SETUP, 2, "2err%d", rc);
- }
qeth_qdio_clear_card(card, 0);
qeth_clear_qdio_buffers(card);
qeth_clear_working_pool_list(card);
@@ -2352,7 +2312,6 @@ static int qeth_l3_stop_card(struct qeth
qeth_clear_cmd_buffers(&card->read);
qeth_clear_cmd_buffers(&card->write);
}
- card->use_hard_stop = 0;
return rc;
}
@@ -3483,17 +3442,15 @@ static void qeth_l3_remove_device(struct
qeth_set_allowed_threads(card, 0, 1);
wait_event(card->wait_q, qeth_threads_running(card, 0xffffffff) == 0);
- if (cgdev->state == CCWGROUP_ONLINE) {
- card->use_hard_stop = 1;
+ if (cgdev->state == CCWGROUP_ONLINE)
qeth_l3_set_offline(cgdev);
- }
if (card->dev) {
unregister_netdev(card->dev);
card->dev = NULL;
}
- qeth_l3_clear_ip_list(card, 0, 0);
+ qeth_l3_clear_ip_list(card, 0);
qeth_l3_clear_ipato_list(card);
return;
}
@@ -3594,7 +3551,6 @@ contin:
mutex_unlock(&card->discipline_mutex);
return 0;
out_remove:
- card->use_hard_stop = 1;
qeth_l3_stop_card(card, 0);
ccw_device_set_offline(CARD_DDEV(card));
ccw_device_set_offline(CARD_WDEV(card));
@@ -3663,7 +3619,6 @@ static int qeth_l3_recover(void *ptr)
QETH_CARD_TEXT(card, 2, "recover2");
dev_warn(&card->gdev->dev,
"A recovery process has been started for the device\n");
- card->use_hard_stop = 1;
__qeth_l3_set_offline(card->gdev, 1);
rc = __qeth_l3_set_online(card->gdev, 1);
if (!rc)
@@ -3684,7 +3639,6 @@ static int qeth_l3_recover(void *ptr)
static void qeth_l3_shutdown(struct ccwgroup_device *gdev)
{
struct qeth_card *card = dev_get_drvdata(&gdev->dev);
- qeth_l3_clear_ip_list(card, 0, 0);
qeth_qdio_clear_card(card, 0);
qeth_clear_qdio_buffers(card);
}
@@ -3700,7 +3654,6 @@ static int qeth_l3_pm_suspend(struct ccw
if (gdev->state == CCWGROUP_OFFLINE)
return 0;
if (card->state == CARD_STATE_UP) {
- card->use_hard_stop = 1;
__qeth_l3_set_offline(card->gdev, 1);
} else
__qeth_l3_set_offline(card->gdev, 0);
^ permalink raw reply
* [patch 0/1] s390: one more qeth patches for net-next
From: frank.blaschka @ 2011-02-18 14:22 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390
Hi Dave,
one more qeth patch for net-next
shortlog:
Ursula Braun (1)
qeth: remove needless IPA-commands in offline
Thanks,
Frank
^ permalink raw reply
* Re: [PATCH v6 0/9] net: Unified offload configuration
From: Michał Mirosław @ 2011-02-18 14:22 UTC (permalink / raw)
To: David Miller; +Cc: netdev, bhutchings
In-Reply-To: <20110217.145611.229745070.davem@davemloft.net>
On Thu, Feb 17, 2011 at 02:56:11PM -0800, David Miller wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Wed, 16 Feb 2011 03:59:16 +0100 (CET)
> > Here's a v6 of the ethtool unification patch series.
> >
> > What's in it?
> > 1..4:
> > cleanups for the core patches
> > 5:
> > the patch - implement unified ethtool setting ops
> > 6..7:
> > implement interoperation between old and new ethtool ops
> > 8:
> > include RX checksum in features and plug it into new framework
> > 9:
> > convert loopback device to new framework
> >
> > What is it good for?
> > - unifies driver behaviour wrt hardware offloads
> > - removes a lot of boilerplate code from drivers
> > - allows better fine-grained control over used offloads
> Applied to net-next-2.6, please send any bug fixes relative to this.
>
> Please get rid of that annoying message spit out by netif_features_change(),
> it's just spam. If we want notifications for stuff like this, use a
> non-unicast netlink message so those who want to hear it can do so.
You mean netdev_update_features() "Features changed" message? Is it ok
to just demote it to DEBUG level or you want to remove it altogether?
What about netdev_fix_features() messages?
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [patch net-next-2.6] net: convert bonding to use rx_handler
From: Jiri Pirko @ 2011-02-18 14:14 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, davem, shemminger, kaber, fubar, nicolas.2p.debian, andy
In-Reply-To: <1298035791.6201.56.camel@edumazet-laptop>
Fri, Feb 18, 2011 at 02:29:51PM CET, eric.dumazet@gmail.com wrote:
>Le vendredi 18 février 2011 à 14:25 +0100, Jiri Pirko a écrit :
>> This patch converts bonding to use rx_handler. Results in cleaner
>> __netif_receive_skb() with much less exceptions needed. Also bond-specific
>> work is moved into bond code.
>>
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> ---
>> drivers/net/bonding/bond_main.c | 75 ++++++++++++++++++++-
>> include/linux/skbuff.h | 2 +
>> net/core/dev.c | 144 +++++++++++---------------------------
>> net/core/skbuff.c | 1 +
>> 4 files changed, 119 insertions(+), 103 deletions(-)
>>
>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 31f02d0..9f3af5d 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -267,6 +267,7 @@ typedef unsigned char *sk_buff_data_t;
>> * @sk: Socket we are owned by
>> * @tstamp: Time we arrived
>> * @dev: Device we arrived on/are leaving by
>> + * @input_dev: Original device on which we arrived
>> * @transport_header: Transport layer header
>> * @network_header: Network layer header
>> * @mac_header: Link layer header
>> @@ -325,6 +326,7 @@ struct sk_buff {
>>
>> struct sock *sk;
>> struct net_device *dev;
>> + struct net_device *input_dev;
>>
>
>Your patch looks fine, but adding 8 bytes to sk_buff for a "cleanup" is
>really a show stopper for me.
Do not know how to do it better. As for percpu variable, not only
origdev would have to be remembered but also probably skb pointer to
know if it's the first run on the skb or not. Can't really figure out a
better solution. Can you?
Thanks.
Jirka
>
>
>
^ permalink raw reply
* [PATCH v2] net: provide default_advmss() methods to blackhole dst_ops
From: Eric Dumazet @ 2011-02-18 13:44 UTC (permalink / raw)
To: Changli Gao
Cc: George Spelvin, David Miller, linux-kernel, netdev, Roland Dreier,
Daniel Baluta
In-Reply-To: <1298036005.6201.58.camel@edumazet-laptop>
Le vendredi 18 février 2011 à 14:33 +0100, Eric Dumazet a écrit :
> I had this exact idea but found we need struct net pointer to get this
> value, not provided in parameters, so I falled back to the 256 value.
>
>
Hmm, reading again this stuff, maybe we can just use
ipv4_default_advmss() instead of a custom one.
dst->dev should be available
[PATCH] net: provide default_advmss() methods to blackhole dst_ops
Commit 0dbaee3b37e118a (net: Abstract default ADVMSS behind an
accessor.) introduced a possible crash in tcp_connect_init(), when
dst->default_advmss() is called from dst_metric_advmss()
Reported-by: George Spelvin <linux@horizon.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Roland Dreier <roland@purestorage.com>
CC: Changli Gao <xiaosuo@gmail.com>
CC: Daniel Baluta <dbaluta@ixiacom.com>
---
net/ipv4/route.c | 1 +
net/ipv6/route.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 788a3e7..6ed6603 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2722,6 +2722,7 @@ static struct dst_ops ipv4_dst_blackhole_ops = {
.destroy = ipv4_dst_destroy,
.check = ipv4_blackhole_dst_check,
.default_mtu = ipv4_blackhole_default_mtu,
+ .default_advmss = ipv4_default_advmss,
.update_pmtu = ipv4_rt_blackhole_update_pmtu,
};
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 1c29f95..a998db6 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -128,6 +128,7 @@ static struct dst_ops ip6_dst_blackhole_ops = {
.destroy = ip6_dst_destroy,
.check = ip6_dst_check,
.default_mtu = ip6_blackhole_default_mtu,
+ .default_advmss = ip6_default_advmss,
.update_pmtu = ip6_rt_blackhole_update_pmtu,
};
^ permalink raw reply related
* Re: [PATCH] net: Add default_advmss() methods to blackhole dst_ops
From: Daniel Baluta @ 2011-02-18 13:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: Changli Gao, George Spelvin, David Miller, linux-kernel, netdev,
Roland Dreier
In-Reply-To: <1298036005.6201.58.camel@edumazet-laptop>
On Fri, Feb 18, 2011 at 3:33 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le vendredi 18 février 2011 à 21:29 +0800, Changli Gao a écrit :
>> On Fri, Feb 18, 2011 at 9:24 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > Le vendredi 18 février 2011 à 21:16 +0800, Changli Gao a écrit :
>> >
>> >> I am wondering why magic number 256 is used here. Is there a special
>> >> reason? Thanks.
>> >>
>> >
>> > It really doesnt matter. SYN message will be dropped anyway.
>> >
>> > 256 happens to be the default value
>> > of /proc/sys/net/ipv4/route/min_adv_mss
Perhaps a comment would make sense then.
thanks,
Daniel.
^ permalink raw reply
* Re: [PATCH] net: Add default_advmss() methods to blackhole dst_ops
From: Eric Dumazet @ 2011-02-18 13:33 UTC (permalink / raw)
To: Changli Gao
Cc: George Spelvin, David Miller, linux-kernel, netdev, Roland Dreier
In-Reply-To: <AANLkTi=O1LZzSc8=qMX=R3rJpCVL8eWr1eo6NkiZXS0m@mail.gmail.com>
Le vendredi 18 février 2011 à 21:29 +0800, Changli Gao a écrit :
> On Fri, Feb 18, 2011 at 9:24 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le vendredi 18 février 2011 à 21:16 +0800, Changli Gao a écrit :
> >
> >> I am wondering why magic number 256 is used here. Is there a special
> >> reason? Thanks.
> >>
> >
> > It really doesnt matter. SYN message will be dropped anyway.
> >
> > 256 happens to be the default value
> > of /proc/sys/net/ipv4/route/min_adv_mss
> >
>
> Thanks for your explaining. IMHO, ip_rt_min_advmss is better than a
> magic number, here.
>
I had this exact idea but found we need struct net pointer to get this
value, not provided in parameters, so I falled back to the 256 value.
^ permalink raw reply
* Re: KIND
From: Mr.David Gurupatham @ 2011-02-18 9:07 UTC (permalink / raw)
To: netdev@vger.kernel.org
My name is David Gurupatham a legal practitioner with David Gurupatham & Associates
in Kuala Lumpur Msia.
I found your contact/profile some where over the Internet and it gave me the
greatest joy, that you are the one I have been looking for. Whom I strongly believe could
execute this project with me. Kindly get back to me for more information
Best regards,
Mr David Gurupatham{Esq}
^ permalink raw reply
* Re: [patch net-next-2.6] net: convert bonding to use rx_handler
From: Eric Dumazet @ 2011-02-18 13:29 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, shemminger, kaber, fubar, nicolas.2p.debian, andy
In-Reply-To: <20110218132524.GC2939@psychotron.redhat.com>
Le vendredi 18 février 2011 à 14:25 +0100, Jiri Pirko a écrit :
> This patch converts bonding to use rx_handler. Results in cleaner
> __netif_receive_skb() with much less exceptions needed. Also bond-specific
> work is moved into bond code.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> drivers/net/bonding/bond_main.c | 75 ++++++++++++++++++++-
> include/linux/skbuff.h | 2 +
> net/core/dev.c | 144 +++++++++++---------------------------
> net/core/skbuff.c | 1 +
> 4 files changed, 119 insertions(+), 103 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 31f02d0..9f3af5d 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -267,6 +267,7 @@ typedef unsigned char *sk_buff_data_t;
> * @sk: Socket we are owned by
> * @tstamp: Time we arrived
> * @dev: Device we arrived on/are leaving by
> + * @input_dev: Original device on which we arrived
> * @transport_header: Transport layer header
> * @network_header: Network layer header
> * @mac_header: Link layer header
> @@ -325,6 +326,7 @@ struct sk_buff {
>
> struct sock *sk;
> struct net_device *dev;
> + struct net_device *input_dev;
>
Your patch looks fine, but adding 8 bytes to sk_buff for a "cleanup" is
really a show stopper for me.
^ permalink raw reply
* Re: [PATCH] net: Add default_advmss() methods to blackhole dst_ops
From: Changli Gao @ 2011-02-18 13:29 UTC (permalink / raw)
To: Eric Dumazet
Cc: George Spelvin, David Miller, linux-kernel, netdev, Roland Dreier
In-Reply-To: <1298035443.6201.51.camel@edumazet-laptop>
On Fri, Feb 18, 2011 at 9:24 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le vendredi 18 février 2011 à 21:16 +0800, Changli Gao a écrit :
>
>> I am wondering why magic number 256 is used here. Is there a special
>> reason? Thanks.
>>
>
> It really doesnt matter. SYN message will be dropped anyway.
>
> 256 happens to be the default value
> of /proc/sys/net/ipv4/route/min_adv_mss
>
Thanks for your explaining. IMHO, ip_rt_min_advmss is better than a
magic number, here.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* [patch net-next-2.6] net: convert bonding to use rx_handler
From: Jiri Pirko @ 2011-02-18 13:25 UTC (permalink / raw)
To: netdev
Cc: davem, shemminger, kaber, fubar, eric.dumazet, nicolas.2p.debian,
andy
This patch converts bonding to use rx_handler. Results in cleaner
__netif_receive_skb() with much less exceptions needed. Also bond-specific
work is moved into bond code.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/bonding/bond_main.c | 75 ++++++++++++++++++++-
include/linux/skbuff.h | 2 +
net/core/dev.c | 144 +++++++++++---------------------------
net/core/skbuff.c | 1 +
4 files changed, 119 insertions(+), 103 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 77e3c6a..a856a11 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1423,6 +1423,68 @@ static void bond_setup_by_slave(struct net_device *bond_dev,
bond->setup_by_slave = 1;
}
+/* On bonding slaves other than the currently active slave, suppress
+ * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and
+ * ARP on active-backup slaves with arp_validate enabled.
+ */
+static bool bond_should_deliver_exact_match(struct sk_buff *skb,
+ struct net_device *slave_dev,
+ struct net_device *bond_dev)
+{
+ if (slave_dev->priv_flags & IFF_SLAVE_INACTIVE) {
+ if (slave_dev->priv_flags & IFF_SLAVE_NEEDARP &&
+ skb->protocol == __cpu_to_be16(ETH_P_ARP))
+ return false;
+
+ if (bond_dev->priv_flags & IFF_MASTER_ALB &&
+ skb->pkt_type != PACKET_BROADCAST &&
+ skb->pkt_type != PACKET_MULTICAST)
+ return false;
+
+ if (bond_dev->priv_flags & IFF_MASTER_8023AD &&
+ skb->protocol == __cpu_to_be16(ETH_P_SLOW))
+ return false;
+
+ return true;
+ }
+ return false;
+}
+
+static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
+{
+ struct net_device *slave_dev;
+ struct net_device *bond_dev;
+
+ skb = skb_share_check(skb, GFP_ATOMIC);
+ if (unlikely(!skb))
+ return NULL;
+ slave_dev = skb->dev;
+ bond_dev = ACCESS_ONCE(slave_dev->master);
+ if (unlikely(!bond_dev))
+ return skb;
+
+ if (bond_dev->priv_flags & IFF_MASTER_ARPMON)
+ slave_dev->last_rx = jiffies;
+
+ if (bond_should_deliver_exact_match(skb, slave_dev, bond_dev)) {
+ skb->deliver_no_wcard = 1;
+ return skb;
+ }
+
+ skb->dev = bond_dev;
+
+ if (bond_dev->priv_flags & IFF_MASTER_ALB &&
+ bond_dev->priv_flags & IFF_BRIDGE_PORT &&
+ skb->pkt_type == PACKET_HOST) {
+ u16 *dest = (u16 *) eth_hdr(skb)->h_dest;
+
+ memcpy(dest, bond_dev->dev_addr, ETH_ALEN);
+ }
+
+ netif_rx(skb);
+ return NULL;
+}
+
/* enslave device <slave> to bond device <master> */
int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
{
@@ -1599,11 +1661,17 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
pr_debug("Error %d calling netdev_set_bond_master\n", res);
goto err_restore_mac;
}
+ res = netdev_rx_handler_register(slave_dev, bond_handle_frame, NULL);
+ if (res) {
+ pr_debug("Error %d calling netdev_rx_handler_register\n", res);
+ goto err_unset_master;
+ }
+
/* open the slave since the application closed it */
res = dev_open(slave_dev);
if (res) {
pr_debug("Opening slave %s failed\n", slave_dev->name);
- goto err_unset_master;
+ goto err_unreg_rxhandler;
}
new_slave->dev = slave_dev;
@@ -1811,6 +1879,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
err_close:
dev_close(slave_dev);
+err_unreg_rxhandler:
+ netdev_rx_handler_unregister(slave_dev);
+
err_unset_master:
netdev_set_bond_master(slave_dev, NULL);
@@ -1992,6 +2063,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
netif_addr_unlock_bh(bond_dev);
}
+ netdev_rx_handler_unregister(slave_dev);
netdev_set_bond_master(slave_dev, NULL);
#ifdef CONFIG_NET_POLL_CONTROLLER
@@ -2114,6 +2186,7 @@ static int bond_release_all(struct net_device *bond_dev)
netif_addr_unlock_bh(bond_dev);
}
+ netdev_rx_handler_unregister(slave_dev);
netdev_set_bond_master(slave_dev, NULL);
/* close slave before restoring its mac address */
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 31f02d0..9f3af5d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -267,6 +267,7 @@ typedef unsigned char *sk_buff_data_t;
* @sk: Socket we are owned by
* @tstamp: Time we arrived
* @dev: Device we arrived on/are leaving by
+ * @input_dev: Original device on which we arrived
* @transport_header: Transport layer header
* @network_header: Network layer header
* @mac_header: Link layer header
@@ -325,6 +326,7 @@ struct sk_buff {
struct sock *sk;
struct net_device *dev;
+ struct net_device *input_dev;
/*
* This is the control buffer. It is free to use for every
diff --git a/net/core/dev.c b/net/core/dev.c
index 4f69439..9d2f485 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1530,12 +1530,17 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(dev_forward_skb);
+static inline int __deliver_skb(struct sk_buff *skb,
+ struct packet_type *pt_prev)
+{
+ return pt_prev->func(skb, skb->dev, pt_prev, skb->input_dev);
+}
+
static inline int deliver_skb(struct sk_buff *skb,
- struct packet_type *pt_prev,
- struct net_device *orig_dev)
+ struct packet_type *pt_prev)
{
atomic_inc(&skb->users);
- return pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
+ return __deliver_skb(skb, pt_prev);
}
/*
@@ -1558,7 +1563,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
(ptype->af_packet_priv == NULL ||
(struct sock *)ptype->af_packet_priv != skb->sk)) {
if (pt_prev) {
- deliver_skb(skb2, pt_prev, skb->dev);
+ deliver_skb(skb2, pt_prev);
pt_prev = ptype;
continue;
}
@@ -1591,7 +1596,7 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
}
}
if (pt_prev)
- pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
+ __deliver_skb(skb2, pt_prev);
rcu_read_unlock();
}
@@ -3021,8 +3026,7 @@ static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
}
static inline struct sk_buff *handle_ing(struct sk_buff *skb,
- struct packet_type **pt_prev,
- int *ret, struct net_device *orig_dev)
+ struct packet_type **pt_prev, int *ret)
{
struct netdev_queue *rxq = rcu_dereference(skb->dev->ingress_queue);
@@ -3030,7 +3034,7 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb,
goto out;
if (*pt_prev) {
- *ret = deliver_skb(skb, *pt_prev, orig_dev);
+ *ret = deliver_skb(skb, *pt_prev);
*pt_prev = NULL;
}
@@ -3092,63 +3096,30 @@ void netdev_rx_handler_unregister(struct net_device *dev)
}
EXPORT_SYMBOL_GPL(netdev_rx_handler_unregister);
-static inline void skb_bond_set_mac_by_master(struct sk_buff *skb,
- struct net_device *master)
-{
- if (skb->pkt_type == PACKET_HOST) {
- u16 *dest = (u16 *) eth_hdr(skb)->h_dest;
-
- memcpy(dest, master->dev_addr, ETH_ALEN);
- }
-}
-
-/* On bonding slaves other than the currently active slave, suppress
- * duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and
- * ARP on active-backup slaves with arp_validate enabled.
- */
-static int __skb_bond_should_drop(struct sk_buff *skb,
- struct net_device *master)
+static void vlan_on_bond_hook(struct sk_buff *skb)
{
- struct net_device *dev = skb->dev;
-
- if (master->priv_flags & IFF_MASTER_ARPMON)
- dev->last_rx = jiffies;
-
- if ((master->priv_flags & IFF_MASTER_ALB) &&
- (master->priv_flags & IFF_BRIDGE_PORT)) {
- /* Do address unmangle. The local destination address
- * will be always the one master has. Provides the right
- * functionality in a bridge.
- */
- skb_bond_set_mac_by_master(skb, master);
- }
-
- if (dev->priv_flags & IFF_SLAVE_INACTIVE) {
- if ((dev->priv_flags & IFF_SLAVE_NEEDARP) &&
- skb->protocol == __cpu_to_be16(ETH_P_ARP))
- return 0;
-
- if (master->priv_flags & IFF_MASTER_ALB) {
- if (skb->pkt_type != PACKET_BROADCAST &&
- skb->pkt_type != PACKET_MULTICAST)
- return 0;
- }
- if (master->priv_flags & IFF_MASTER_8023AD &&
- skb->protocol == __cpu_to_be16(ETH_P_SLOW))
- return 0;
+ /*
+ * Make sure ARP frames received on VLAN interfaces stacked on
+ * bonding interfaces still make their way to any base bonding
+ * device that may have registered for a specific ptype.
+ */
+ if (skb->dev->priv_flags & IFF_802_1Q_VLAN &&
+ vlan_dev_real_dev(skb->dev)->priv_flags & IFF_BONDING &&
+ skb->protocol == htons(ETH_P_ARP)) {
+ struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
- return 1;
+ if (!skb2)
+ return;
+ skb2->dev = vlan_dev_real_dev(skb->dev);
+ netif_rx(skb2);
}
- return 0;
}
static int __netif_receive_skb(struct sk_buff *skb)
{
struct packet_type *ptype, *pt_prev;
rx_handler_func_t *rx_handler;
- struct net_device *orig_dev;
- struct net_device *null_or_orig;
- struct net_device *orig_or_bond;
+ struct net_device *null_or_dev;
int ret = NET_RX_DROP;
__be16 type;
@@ -3164,29 +3135,8 @@ static int __netif_receive_skb(struct sk_buff *skb)
if (!skb->skb_iif)
skb->skb_iif = skb->dev->ifindex;
- /*
- * bonding note: skbs received on inactive slaves should only
- * be delivered to pkt handlers that are exact matches. Also
- * the deliver_no_wcard flag will be set. If packet handlers
- * are sensitive to duplicate packets these skbs will need to
- * be dropped at the handler.
- */
- null_or_orig = NULL;
- orig_dev = skb->dev;
- if (skb->deliver_no_wcard)
- null_or_orig = orig_dev;
- else if (netif_is_bond_slave(orig_dev)) {
- struct net_device *bond_master = ACCESS_ONCE(orig_dev->master);
-
- if (likely(bond_master)) {
- if (__skb_bond_should_drop(skb, bond_master)) {
- skb->deliver_no_wcard = 1;
- /* deliver only exact match */
- null_or_orig = orig_dev;
- } else
- skb->dev = bond_master;
- }
- }
+ if (!skb->input_dev)
+ skb->input_dev = skb->dev;
__this_cpu_inc(softnet_data.processed);
skb_reset_network_header(skb);
@@ -3205,26 +3155,24 @@ static int __netif_receive_skb(struct sk_buff *skb)
#endif
list_for_each_entry_rcu(ptype, &ptype_all, list) {
- if (ptype->dev == null_or_orig || ptype->dev == skb->dev ||
- ptype->dev == orig_dev) {
+ if (!ptype->dev || ptype->dev == skb->dev) {
if (pt_prev)
- ret = deliver_skb(skb, pt_prev, orig_dev);
+ ret = deliver_skb(skb, pt_prev);
pt_prev = ptype;
}
}
#ifdef CONFIG_NET_CLS_ACT
- skb = handle_ing(skb, &pt_prev, &ret, orig_dev);
+ skb = handle_ing(skb, &pt_prev, &ret);
if (!skb)
goto out;
ncls:
#endif
- /* Handle special case of bridge or macvlan */
rx_handler = rcu_dereference(skb->dev->rx_handler);
if (rx_handler) {
if (pt_prev) {
- ret = deliver_skb(skb, pt_prev, orig_dev);
+ ret = deliver_skb(skb, pt_prev);
pt_prev = NULL;
}
skb = rx_handler(skb);
@@ -3234,7 +3182,7 @@ ncls:
if (vlan_tx_tag_present(skb)) {
if (pt_prev) {
- ret = deliver_skb(skb, pt_prev, orig_dev);
+ ret = deliver_skb(skb, pt_prev);
pt_prev = NULL;
}
if (vlan_hwaccel_do_receive(&skb)) {
@@ -3244,32 +3192,24 @@ ncls:
goto out;
}
- /*
- * Make sure frames received on VLAN interfaces stacked on
- * bonding interfaces still make their way to any base bonding
- * device that may have registered for a specific ptype. The
- * handler may have to adjust skb->dev and orig_dev.
- */
- orig_or_bond = orig_dev;
- if ((skb->dev->priv_flags & IFF_802_1Q_VLAN) &&
- (vlan_dev_real_dev(skb->dev)->priv_flags & IFF_BONDING)) {
- orig_or_bond = vlan_dev_real_dev(skb->dev);
- }
+ vlan_on_bond_hook(skb);
+
+ /* deliver only exact match when indicated */
+ null_or_dev = skb->deliver_no_wcard ? skb->dev : NULL;
type = skb->protocol;
list_for_each_entry_rcu(ptype,
&ptype_base[ntohs(type) & PTYPE_HASH_MASK], list) {
- if (ptype->type == type && (ptype->dev == null_or_orig ||
- ptype->dev == skb->dev || ptype->dev == orig_dev ||
- ptype->dev == orig_or_bond)) {
+ if (ptype->type == type &&
+ (ptype->dev == null_or_dev || ptype->dev == skb->dev)) {
if (pt_prev)
- ret = deliver_skb(skb, pt_prev, orig_dev);
+ ret = deliver_skb(skb, pt_prev);
pt_prev = ptype;
}
}
if (pt_prev) {
- ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
+ ret = __deliver_skb(skb, pt_prev);
} else {
atomic_long_inc(&skb->dev->rx_dropped);
kfree_skb(skb);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 14cf560..e19eabe 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -508,6 +508,7 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
{
new->tstamp = old->tstamp;
new->dev = old->dev;
+ new->input_dev = old->input_dev;
new->transport_header = old->transport_header;
new->network_header = old->network_header;
new->mac_header = old->mac_header;
--
1.7.3.4
^ permalink raw reply related
* [PATCH net-next-2.6] net: add __rcu annotations to sk_wq and wq
From: Eric Dumazet @ 2011-02-18 13:26 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Add proper RCU annotations/verbs to sk_wq and wq members
Fix __sctp_write_space() sk_sleep() abuse (and sock->wq access)
Fix sunrpc sk_sleep() abuse too
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/net.h | 3 ++-
include/net/sock.h | 7 ++++---
net/sctp/socket.c | 9 +++++----
net/socket.c | 23 ++++++++++++++---------
net/sunrpc/svcsock.c | 32 ++++++++++++++++++++------------
net/unix/af_unix.c | 2 +-
6 files changed, 46 insertions(+), 30 deletions(-)
diff --git a/include/linux/net.h b/include/linux/net.h
index 16faa13..94de83c 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -118,6 +118,7 @@ enum sock_shutdown_cmd {
};
struct socket_wq {
+ /* Note: wait MUST be first field of socket_wq */
wait_queue_head_t wait;
struct fasync_struct *fasync_list;
struct rcu_head rcu;
@@ -142,7 +143,7 @@ struct socket {
unsigned long flags;
- struct socket_wq *wq;
+ struct socket_wq __rcu *wq;
struct file *file;
struct sock *sk;
diff --git a/include/net/sock.h b/include/net/sock.h
index e3893a2..da0534d 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -281,7 +281,7 @@ struct sock {
int sk_rcvbuf;
struct sk_filter __rcu *sk_filter;
- struct socket_wq *sk_wq;
+ struct socket_wq __rcu *sk_wq;
#ifdef CONFIG_NET_DMA
struct sk_buff_head sk_async_wait_queue;
@@ -1266,7 +1266,8 @@ static inline void sk_set_socket(struct sock *sk, struct socket *sock)
static inline wait_queue_head_t *sk_sleep(struct sock *sk)
{
- return &sk->sk_wq->wait;
+ BUILD_BUG_ON(offsetof(struct socket_wq, wait) != 0);
+ return &rcu_dereference_raw(sk->sk_wq)->wait;
}
/* Detach socket from process context.
* Announce socket dead, detach it from wait queue and inode.
@@ -1287,7 +1288,7 @@ static inline void sock_orphan(struct sock *sk)
static inline void sock_graft(struct sock *sk, struct socket *parent)
{
write_lock_bh(&sk->sk_callback_lock);
- rcu_assign_pointer(sk->sk_wq, parent->wq);
+ sk->sk_wq = parent->wq;
parent->sk = sk;
sk_set_socket(sk, parent);
security_sock_graft(sk, parent);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 8e02550..b53b2eb 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6102,15 +6102,16 @@ static void __sctp_write_space(struct sctp_association *asoc)
wake_up_interruptible(&asoc->wait);
if (sctp_writeable(sk)) {
- if (sk_sleep(sk) && waitqueue_active(sk_sleep(sk)))
- wake_up_interruptible(sk_sleep(sk));
+ wait_queue_head_t *wq = sk_sleep(sk);
+
+ if (wq && waitqueue_active(wq))
+ wake_up_interruptible(wq);
/* Note that we try to include the Async I/O support
* here by modeling from the current TCP/UDP code.
* We have not tested with it yet.
*/
- if (sock->wq->fasync_list &&
- !(sk->sk_shutdown & SEND_SHUTDOWN))
+ if (!(sk->sk_shutdown & SEND_SHUTDOWN))
sock_wake_async(sock,
SOCK_WAKE_SPACE, POLL_OUT);
}
diff --git a/net/socket.c b/net/socket.c
index ac2219f..9fa1e3b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -240,17 +240,19 @@ static struct kmem_cache *sock_inode_cachep __read_mostly;
static struct inode *sock_alloc_inode(struct super_block *sb)
{
struct socket_alloc *ei;
+ struct socket_wq *wq;
ei = kmem_cache_alloc(sock_inode_cachep, GFP_KERNEL);
if (!ei)
return NULL;
- ei->socket.wq = kmalloc(sizeof(struct socket_wq), GFP_KERNEL);
- if (!ei->socket.wq) {
+ wq = kmalloc(sizeof(*wq), GFP_KERNEL);
+ if (!wq) {
kmem_cache_free(sock_inode_cachep, ei);
return NULL;
}
- init_waitqueue_head(&ei->socket.wq->wait);
- ei->socket.wq->fasync_list = NULL;
+ init_waitqueue_head(&wq->wait);
+ wq->fasync_list = NULL;
+ RCU_INIT_POINTER(ei->socket.wq, wq);
ei->socket.state = SS_UNCONNECTED;
ei->socket.flags = 0;
@@ -273,9 +275,11 @@ static void wq_free_rcu(struct rcu_head *head)
static void sock_destroy_inode(struct inode *inode)
{
struct socket_alloc *ei;
+ struct socket_wq *wq;
ei = container_of(inode, struct socket_alloc, vfs_inode);
- call_rcu(&ei->socket.wq->rcu, wq_free_rcu);
+ wq = rcu_dereference_protected(ei->socket.wq, 1);
+ call_rcu(&wq->rcu, wq_free_rcu);
kmem_cache_free(sock_inode_cachep, ei);
}
@@ -524,7 +528,7 @@ void sock_release(struct socket *sock)
module_put(owner);
}
- if (sock->wq->fasync_list)
+ if (rcu_dereference_protected(sock->wq, 1)->fasync_list)
printk(KERN_ERR "sock_release: fasync list not empty!\n");
percpu_sub(sockets_in_use, 1);
@@ -1108,15 +1112,16 @@ static int sock_fasync(int fd, struct file *filp, int on)
{
struct socket *sock = filp->private_data;
struct sock *sk = sock->sk;
+ struct socket_wq *wq;
if (sk == NULL)
return -EINVAL;
lock_sock(sk);
+ wq = rcu_dereference_protected(sock->wq, sock_owned_by_user(sk));
+ fasync_helper(fd, filp, on, &wq->fasync_list);
- fasync_helper(fd, filp, on, &sock->wq->fasync_list);
-
- if (!sock->wq->fasync_list)
+ if (!wq->fasync_list)
sock_reset_flag(sk, SOCK_FASYNC);
else
sock_set_flag(sk, SOCK_FASYNC);
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 7bd3bbb..76425d2 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -420,6 +420,7 @@ static void svc_sock_setbufsize(struct socket *sock, unsigned int snd,
static void svc_udp_data_ready(struct sock *sk, int count)
{
struct svc_sock *svsk = (struct svc_sock *)sk->sk_user_data;
+ wait_queue_head_t *wq = sk_sleep(sk);
if (svsk) {
dprintk("svc: socket %p(inet %p), count=%d, busy=%d\n",
@@ -428,8 +429,8 @@ static void svc_udp_data_ready(struct sock *sk, int count)
set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags);
svc_xprt_enqueue(&svsk->sk_xprt);
}
- if (sk_sleep(sk) && waitqueue_active(sk_sleep(sk)))
- wake_up_interruptible(sk_sleep(sk));
+ if (wq && waitqueue_active(wq))
+ wake_up_interruptible(wq);
}
/*
@@ -438,6 +439,7 @@ static void svc_udp_data_ready(struct sock *sk, int count)
static void svc_write_space(struct sock *sk)
{
struct svc_sock *svsk = (struct svc_sock *)(sk->sk_user_data);
+ wait_queue_head_t *wq = sk_sleep(sk);
if (svsk) {
dprintk("svc: socket %p(inet %p), write_space busy=%d\n",
@@ -445,10 +447,10 @@ static void svc_write_space(struct sock *sk)
svc_xprt_enqueue(&svsk->sk_xprt);
}
- if (sk_sleep(sk) && waitqueue_active(sk_sleep(sk))) {
+ if (wq && waitqueue_active(wq)) {
dprintk("RPC svc_write_space: someone sleeping on %p\n",
svsk);
- wake_up_interruptible(sk_sleep(sk));
+ wake_up_interruptible(wq);
}
}
@@ -739,6 +741,7 @@ static void svc_udp_init(struct svc_sock *svsk, struct svc_serv *serv)
static void svc_tcp_listen_data_ready(struct sock *sk, int count_unused)
{
struct svc_sock *svsk = (struct svc_sock *)sk->sk_user_data;
+ wait_queue_head_t *wq;
dprintk("svc: socket %p TCP (listen) state change %d\n",
sk, sk->sk_state);
@@ -761,8 +764,9 @@ static void svc_tcp_listen_data_ready(struct sock *sk, int count_unused)
printk("svc: socket %p: no user data\n", sk);
}
- if (sk_sleep(sk) && waitqueue_active(sk_sleep(sk)))
- wake_up_interruptible_all(sk_sleep(sk));
+ wq = sk_sleep(sk);
+ if (wq && waitqueue_active(wq))
+ wake_up_interruptible_all(wq);
}
/*
@@ -771,6 +775,7 @@ static void svc_tcp_listen_data_ready(struct sock *sk, int count_unused)
static void svc_tcp_state_change(struct sock *sk)
{
struct svc_sock *svsk = (struct svc_sock *)sk->sk_user_data;
+ wait_queue_head_t *wq = sk_sleep(sk);
dprintk("svc: socket %p TCP (connected) state change %d (svsk %p)\n",
sk, sk->sk_state, sk->sk_user_data);
@@ -781,13 +786,14 @@ static void svc_tcp_state_change(struct sock *sk)
set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
svc_xprt_enqueue(&svsk->sk_xprt);
}
- if (sk_sleep(sk) && waitqueue_active(sk_sleep(sk)))
- wake_up_interruptible_all(sk_sleep(sk));
+ if (wq && waitqueue_active(wq))
+ wake_up_interruptible_all(wq);
}
static void svc_tcp_data_ready(struct sock *sk, int count)
{
struct svc_sock *svsk = (struct svc_sock *)sk->sk_user_data;
+ wait_queue_head_t *wq = sk_sleep(sk);
dprintk("svc: socket %p TCP data ready (svsk %p)\n",
sk, sk->sk_user_data);
@@ -795,8 +801,8 @@ static void svc_tcp_data_ready(struct sock *sk, int count)
set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags);
svc_xprt_enqueue(&svsk->sk_xprt);
}
- if (sk_sleep(sk) && waitqueue_active(sk_sleep(sk)))
- wake_up_interruptible(sk_sleep(sk));
+ if (wq && waitqueue_active(wq))
+ wake_up_interruptible(wq);
}
/*
@@ -1531,6 +1537,7 @@ static void svc_sock_detach(struct svc_xprt *xprt)
{
struct svc_sock *svsk = container_of(xprt, struct svc_sock, sk_xprt);
struct sock *sk = svsk->sk_sk;
+ wait_queue_head_t *wq;
dprintk("svc: svc_sock_detach(%p)\n", svsk);
@@ -1539,8 +1546,9 @@ static void svc_sock_detach(struct svc_xprt *xprt)
sk->sk_data_ready = svsk->sk_odata;
sk->sk_write_space = svsk->sk_owspace;
- if (sk_sleep(sk) && waitqueue_active(sk_sleep(sk)))
- wake_up_interruptible(sk_sleep(sk));
+ wq = sk_sleep(sk);
+ if (wq && waitqueue_active(wq))
+ wake_up_interruptible(wq);
}
/*
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index d8d98d5..217fb7f 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1171,7 +1171,7 @@ restart:
newsk->sk_type = sk->sk_type;
init_peercred(newsk);
newu = unix_sk(newsk);
- newsk->sk_wq = &newu->peer_wq;
+ RCU_INIT_POINTER(newsk->sk_wq, &newu->peer_wq);
otheru = unix_sk(other);
/* copy address information from listening to new sock*/
^ permalink raw reply related
* Re: [PATCH] net: Add default_advmss() methods to blackhole dst_ops
From: Eric Dumazet @ 2011-02-18 13:24 UTC (permalink / raw)
To: Changli Gao
Cc: George Spelvin, David Miller, linux-kernel, netdev, Roland Dreier
In-Reply-To: <AANLkTi=GRPWNWZuORx1zoJJURE-drqEoMdTuKB9LZ8Ux@mail.gmail.com>
Le vendredi 18 février 2011 à 21:16 +0800, Changli Gao a écrit :
> I am wondering why magic number 256 is used here. Is there a special
> reason? Thanks.
>
It really doesnt matter. SYN message will be dropped anyway.
256 happens to be the default value
of /proc/sys/net/ipv4/route/min_adv_mss
^ permalink raw reply
* Re: [PATCH] net: Add default_advmss() methods to blackhole dst_ops
From: Changli Gao @ 2011-02-18 13:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: George Spelvin, David Miller, linux-kernel, netdev, Roland Dreier
In-Reply-To: <1298023023.2595.170.camel@edumazet-laptop>
On Fri, Feb 18, 2011 at 5:57 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> I suspect following patch is needed
>
> CC Roland Dreier <roland@purestorage.com> because he fixed the
> default_mtu problem in commit ec831ea72ee5d7d47
> (net: Add default_mtu() methods to blackhole dst_ops)
>
> Thanks !
>
> [PATCH] net: Add default_advmss() methods to blackhole dst_ops
>
> Commit 0dbaee3b37e118a (net: Abstract default ADVMSS behind an
> accessor.) introduced a possible crash in tcp_connect_init(), when
> dst->default_advmss() is called from dst_metric_advmss()
>
> Reported-by: George Spelvin <linux@horizon.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Roland Dreier <roland@purestorage.com>
> ---
> net/ipv4/route.c | 7 +++++++
> net/ipv6/route.c | 6 ++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 788a3e7..5edb605 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2712,6 +2712,12 @@ static unsigned int ipv4_blackhole_default_mtu(const struct dst_entry *dst)
> return 0;
> }
>
> +static unsigned int ipv4_blackhole_default_advmss(const struct dst_entry *dst)
> +{
> + return 256;
> +}
> +
> +
I am wondering why magic number 256 is used here. Is there a special
reason? Thanks.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: KIND
From: Mr.David Gurupatham @ 2011-02-18 9:07 UTC (permalink / raw)
To: netdev@vger.kernel.org
My name is David Gurupatham a legal practitioner with David Gurupatham & Associates
in Kuala Lumpur Msia.
I found your contact/profile some where over the Internet and it gave me the
greatest joy, that you are the one I have been looking for. Whom I strongly believe could
execute this project with me. Kindly get back to me for more information
Best regards,
Mr David Gurupatham{Esq}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox