* [PATCH 1/2] net/fsl: drop in_be32() & out_be32() in xgmac_mdio
From: shh.xie @ 2015-01-21 11:07 UTC (permalink / raw)
To: netdev, davem; +Cc: Shaohui Xie
From: Shaohui Xie <Shaohui.Xie@freescale.com>
Use ioread32be() & iowrite32be() instead.
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
drivers/net/ethernet/freescale/xgmac_mdio.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index 3a76e23..ab9a6bf 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -56,7 +56,7 @@ static int xgmac_wait_until_free(struct device *dev,
/* Wait till the bus is free */
status = spin_event_timeout(
- !((in_be32(®s->mdio_stat)) & MDIO_STAT_BSY), TIMEOUT, 0);
+ !((ioread32be(®s->mdio_stat)) & MDIO_STAT_BSY), TIMEOUT, 0);
if (!status) {
dev_err(dev, "timeout waiting for bus to be free\n");
return -ETIMEDOUT;
@@ -75,7 +75,7 @@ static int xgmac_wait_until_done(struct device *dev,
/* Wait till the MDIO write is complete */
status = spin_event_timeout(
- !((in_be32(®s->mdio_data)) & MDIO_DATA_BSY), TIMEOUT, 0);
+ !((ioread32be(®s->mdio_data)) & MDIO_DATA_BSY), TIMEOUT, 0);
if (!status) {
dev_err(dev, "timeout waiting for operation to complete\n");
return -ETIMEDOUT;
@@ -96,7 +96,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
u32 mdio_ctl, mdio_stat;
int ret;
- mdio_stat = in_be32(®s->mdio_stat);
+ mdio_stat = ioread32be(®s->mdio_stat);
if (regnum & MII_ADDR_C45) {
/* Clause 45 (ie 10G) */
dev_addr = (regnum >> 16) & 0x1f;
@@ -107,7 +107,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
mdio_stat &= ~MDIO_STAT_ENC;
}
- out_be32(®s->mdio_stat, mdio_stat);
+ iowrite32be(mdio_stat, ®s->mdio_stat);
ret = xgmac_wait_until_free(&bus->dev, regs);
if (ret)
@@ -115,11 +115,11 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
/* Set the port and dev addr */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
- out_be32(®s->mdio_ctl, mdio_ctl);
+ iowrite32be(mdio_ctl, ®s->mdio_ctl);
/* Set the register address */
if (regnum & MII_ADDR_C45) {
- out_be32(®s->mdio_addr, regnum & 0xffff);
+ iowrite32be(regnum & 0xffff, ®s->mdio_addr);
ret = xgmac_wait_until_free(&bus->dev, regs);
if (ret)
@@ -127,7 +127,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
}
/* Write the value to the register */
- out_be32(®s->mdio_data, MDIO_DATA(value));
+ iowrite32be(MDIO_DATA(value), ®s->mdio_data);
ret = xgmac_wait_until_done(&bus->dev, regs);
if (ret)
@@ -150,7 +150,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
uint16_t value;
int ret;
- mdio_stat = in_be32(®s->mdio_stat);
+ mdio_stat = ioread32be(®s->mdio_stat);
if (regnum & MII_ADDR_C45) {
dev_addr = (regnum >> 16) & 0x1f;
mdio_stat |= MDIO_STAT_ENC;
@@ -159,7 +159,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
mdio_stat &= ~MDIO_STAT_ENC;
}
- out_be32(®s->mdio_stat, mdio_stat);
+ iowrite32be(mdio_stat, ®s->mdio_stat);
ret = xgmac_wait_until_free(&bus->dev, regs);
if (ret)
@@ -167,11 +167,11 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
/* Set the Port and Device Addrs */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
- out_be32(®s->mdio_ctl, mdio_ctl);
+ iowrite32be(mdio_ctl, ®s->mdio_ctl);
/* Set the register address */
if (regnum & MII_ADDR_C45) {
- out_be32(®s->mdio_addr, regnum & 0xffff);
+ iowrite32be(regnum & 0xffff, ®s->mdio_addr);
ret = xgmac_wait_until_free(&bus->dev, regs);
if (ret)
@@ -179,21 +179,21 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
}
/* Initiate the read */
- out_be32(®s->mdio_ctl, mdio_ctl | MDIO_CTL_READ);
+ iowrite32be(mdio_ctl | MDIO_CTL_READ, ®s->mdio_ctl);
ret = xgmac_wait_until_done(&bus->dev, regs);
if (ret)
return ret;
/* Return all Fs if nothing was there */
- if (in_be32(®s->mdio_stat) & MDIO_STAT_RD_ER) {
+ if (ioread32be(®s->mdio_stat) & MDIO_STAT_RD_ER) {
dev_err(&bus->dev,
"Error while reading PHY%d reg at %d.%hhu\n",
phy_id, dev_addr, regnum);
return 0xffff;
}
- value = in_be32(®s->mdio_data) & 0xffff;
+ value = ioread32be(®s->mdio_data) & 0xffff;
dev_dbg(&bus->dev, "read %04x\n", value);
return value;
--
1.8.4.1
^ permalink raw reply related
* [PATCH 2/2] net/fsl: Replace spin_event_timeout() with arch independent in xgmac_mdio
From: shh.xie @ 2015-01-21 11:08 UTC (permalink / raw)
To: netdev, davem; +Cc: Shaohui Xie
From: Shaohui Xie <Shaohui.Xie@freescale.com>
spin_event_timeout() is PPC dependent, use an arch independent
equivalent instead.
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
drivers/net/ethernet/freescale/xgmac_mdio.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index ab9a6bf..3a83bc2 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -52,12 +52,16 @@ struct tgec_mdio_controller {
static int xgmac_wait_until_free(struct device *dev,
struct tgec_mdio_controller __iomem *regs)
{
- uint32_t status;
+ unsigned int timeout;
/* Wait till the bus is free */
- status = spin_event_timeout(
- !((ioread32be(®s->mdio_stat)) & MDIO_STAT_BSY), TIMEOUT, 0);
- if (!status) {
+ timeout = TIMEOUT;
+ while ((ioread32be(®s->mdio_stat) & MDIO_STAT_BSY) && timeout) {
+ cpu_relax();
+ timeout--;
+ }
+
+ if (!timeout) {
dev_err(dev, "timeout waiting for bus to be free\n");
return -ETIMEDOUT;
}
@@ -71,12 +75,16 @@ static int xgmac_wait_until_free(struct device *dev,
static int xgmac_wait_until_done(struct device *dev,
struct tgec_mdio_controller __iomem *regs)
{
- uint32_t status;
+ unsigned int timeout;
/* Wait till the MDIO write is complete */
- status = spin_event_timeout(
- !((ioread32be(®s->mdio_data)) & MDIO_DATA_BSY), TIMEOUT, 0);
- if (!status) {
+ timeout = TIMEOUT;
+ while ((ioread32be(®s->mdio_data) & MDIO_DATA_BSY) && timeout) {
+ cpu_relax();
+ timeout--;
+ }
+
+ if (!timeout) {
dev_err(dev, "timeout waiting for operation to complete\n");
return -ETIMEDOUT;
}
--
1.8.4.1
^ permalink raw reply related
* RE: BW regression after "tcp: refine TSO autosizing"
From: David Laight @ 2015-01-21 12:26 UTC (permalink / raw)
To: 'Rick Jones', Eric Dumazet, Dave Taht
Cc: Eyal Perry, Yuchung Cheng, Neal Cardwell, Eyal Perry, Or Gerlitz,
Linux Netdev List, Amir Vadai, Yevgeny Petrilin, Saeed Mahameed,
Ido Shamay, Amir Ancel
In-Reply-To: <54BEA91D.1050001@hp.com>
From: Of Rick Jones
> >> Are you saying that at long last, delayed acks as we knew them are
> >> dead, dead, dead?
> >
> > Sorry, I can not parse what you are saying.
> >
> > In case you missed it, it has nothing to do with delayed ACK but GRO on
> > receiver.
>
> Dave - assuming I've interpreted Eric's comments correctly, I believe
> the answer to your question is No. Your desire for a world brimming
> with ack-every-other purity has not been fulfilled :)
>
> However, the engineers formerly at Mentat are probably pleased that a
> functional near-equivalent to their ACK avoidance heuristic has ended-up
> being implemented and tacitly accepted, albeit by the back door :)
I must recheck something I discovered a while back with more recent kernels.
There has been a bad interaction between 'slow start' and 'delayed acks'
when nagle is disabled on 0 RTT local links with uni-directional traffic.
'Slow start' would refuse to send more than 4 messages until it received
an ack (rather than 4 mss of data).
The receiving system wouldn't send an ack until the timer expired
(or several mss of data were received) by which time the sender could have
a lot of data queued.
Due to the 0 RTT and bursty nature of the data 'slow start' happened
all the time.
David
^ permalink raw reply
* Re: [PATCH v5 4/5] can: kvaser_usb: Retry the first bulk transfer on -ETIMEDOUT
From: Sergei Shtylyov @ 2015-01-21 12:24 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Marc Kleine-Budde
Cc: Andri Yngvason, Linux-CAN, netdev, LKML
In-Reply-To: <20150120214834.GD16828@linux>
Hello.
On 1/21/2015 12:48 AM, Ahmed S. Darwish wrote:
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> On some x86 laptops, plugging a Kvaser device again after an
> unplug makes the firmware always ignore the very first command.
> For such a case, provide some room for retries instead of
> completly exiting the driver init code.
Completely.
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> ---
> drivers/net/can/usb/kvaser_usb.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index 640b0eb..068e76c 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
[...]
> @@ -1632,7 +1632,15 @@ static int kvaser_usb_probe(struct usb_interface *intf,
>
> usb_set_intfdata(intf, dev);
>
> - err = kvaser_usb_get_software_info(dev);
> + /* On some x86 laptops, plugging a Kvaser device again after
> + * an unplug makes the firmware always ignore the very first
> + * command. For such a case, provide some room for retries
> + * instead of completly exiting the driver.
Completely.
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Hiroshi Shimamoto @ 2015-01-21 12:18 UTC (permalink / raw)
To: David Laight, Skidmore, Donald C, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, Hayato Momma, linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD01D6@AcuExch.aculab.com>
> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
>
> From: Hiroshi Shimamoto
> > My concern is what is the real issue that VF multicast promiscuous mode can cause.
> > I think there is the 4k entries to filter multicast address, and the current ixgbe/ixgbevf
> > can turn all bits on from VM. That is almost same as enabling multicast promiscuous mode.
> > I mean that we can receive all multicast addresses by an onerous operation in untrusted VM.
> > I think we should clarify what is real security issue in this context.
>
> If you are worried about passing un-enabled multicasts to users then
> what about doing a software hash of received multicasts and checking
> against an actual list of multicasts enabled for that hash entry.
> Under normal conditions there is likely to be only a single address to check.
>
> It may (or may not) be best to use the same hash as any hashing hardware
> filter uses.
thanks for the comment. But I don't think that is the point.
I guess, introducing VF multicast promiscuous mode seems to add new privilege
to peek every multicast packet in VM and that doesn't look good.
On the other hand, I think that there has been the same privilege in the current
ixgbe/ixgbevf implementation already. Or I'm reading the code wrongly.
I'd like to clarify what is the issue of allowing to receive all multicast packets.
thanks,
Hiroshi
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH 2/3] netlink: Mark dumps as inconsistent which have been interrupted by a resize
From: Thomas Graf @ 2015-01-21 12:17 UTC (permalink / raw)
To: Ying Xue; +Cc: davem, kaber, herbert, paulmck, netdev, netfilter-devel
In-Reply-To: <54BF5FC4.4070808@windriver.com>
On 01/21/15 at 04:13pm, Ying Xue wrote:
> On 01/20/2015 09:20 PM, Thomas Graf wrote:
> > A deferred resize of nl_table causes the offsets that Netlink diag keeps
> > to become inaccurate. Mark the dump as inconsistent and have user space
> > request a new dump.
> >
> > Signed-off-by: Thomas Graf <tgraf@suug.ch>
> > ---
> > net/netlink/af_netlink.c | 10 ++++++++++
> > net/netlink/af_netlink.h | 1 +
> > net/netlink/diag.c | 1 +
> > 3 files changed, 12 insertions(+)
> >
> > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> > index 7a94185..e214557 100644
> > --- a/net/netlink/af_netlink.c
> > +++ b/net/netlink/af_netlink.c
> > @@ -91,6 +91,9 @@ static inline int netlink_is_kernel(struct sock *sk)
> > struct netlink_table *nl_table;
> > EXPORT_SYMBOL_GPL(nl_table);
> >
> > +atomic_t nl_table_seq;
>
> It sounds like the atomic variable is not initialized.
Thanks for the review. We also need to avoid hitting 0 when we overflow
on a seq increment. The netfilter code is doing this correctly but several
other users are suffering from this as well.
I'll address this in v2 together with the other discussed changes.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Patrick McHardy @ 2015-01-21 12:09 UTC (permalink / raw)
To: Thomas Graf; +Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121120727.GC12570@casper.infradead.org>
On 21.01, Thomas Graf wrote:
> On 01/21/15 at 11:59am, Patrick McHardy wrote:
> > On 21.01, Thomas Graf wrote:
> > > On 01/21/15 at 10:40am, Patrick McHardy wrote:
> > > > An automatic restart handles this well. Userspace always had to
> > > > expect duplicates.
> > >
> > > Maybe I don't understand the restart yet. How do you restart if the
> > > dump was already started and the user has read part of the dump?
> >
> > You use the mutex to prevent concurrent resizes and dumps, for dumps
> > that have left the kernel you simply reset the iterator state after
> > a resize operation.
>
> I see, so we possibly see a lot of duplicates but we will never miss
> any entries.
>
> So in summary we do:
>
> 2) We allow rhashtable users to opt in to Herbert's consistent walker
> relying on the use of an additional bit.
>
> 2) If user supports NLM_F_DUMP_INTR we block out resizes during
> message construction and set NLM_F_DUMP_INTR if a resize/rehash
> interrupted us while the user read.
>
> 3) If (1) and (2) are not available we do (2) but restart the dump
> on interruption thus causing duplicates but never missing any
> entries.
>
> I think this offers good flexibility for new users and compatibility
> for existing users.
Yep, sounds good to me.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-21 12:07 UTC (permalink / raw)
To: Patrick McHardy
Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121115924.GW3012@acer.localdomain>
On 01/21/15 at 11:59am, Patrick McHardy wrote:
> On 21.01, Thomas Graf wrote:
> > On 01/21/15 at 10:40am, Patrick McHardy wrote:
> > > An automatic restart handles this well. Userspace always had to
> > > expect duplicates.
> >
> > Maybe I don't understand the restart yet. How do you restart if the
> > dump was already started and the user has read part of the dump?
>
> You use the mutex to prevent concurrent resizes and dumps, for dumps
> that have left the kernel you simply reset the iterator state after
> a resize operation.
I see, so we possibly see a lot of duplicates but we will never miss
any entries.
So in summary we do:
2) We allow rhashtable users to opt in to Herbert's consistent walker
relying on the use of an additional bit.
2) If user supports NLM_F_DUMP_INTR we block out resizes during
message construction and set NLM_F_DUMP_INTR if a resize/rehash
interrupted us while the user read.
3) If (1) and (2) are not available we do (2) but restart the dump
on interruption thus causing duplicates but never missing any
entries.
I think this offers good flexibility for new users and compatibility
for existing users.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Patrick McHardy @ 2015-01-21 11:59 UTC (permalink / raw)
To: Thomas Graf; +Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121113751.GT20315@casper.infradead.org>
On 21.01, Thomas Graf wrote:
> On 01/21/15 at 10:40am, Patrick McHardy wrote:
> > On the danger of repeating myself, every (converted) user requires
> > that we at least keep the existing semantics since it is exposed to
> > userspace. My opinion is that NLM_F_DUMP_INTR is fine if userspace
> > indicates support, but without that, we have to take care of that
> > in the kernel.
>
> Absolutely agreed. I think this is an excellent low cost path for
> future users where dumps are rare.
>
> > An automatic restart handles this well. Userspace always had to
> > expect duplicates.
>
> Maybe I don't understand the restart yet. How do you restart if the
> dump was already started and the user has read part of the dump?
You use the mutex to prevent concurrent resizes and dumps, for dumps
that have left the kernel you simply reset the iterator state after
a resize operation.
^ permalink raw reply
* RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: David Laight @ 2015-01-21 11:56 UTC (permalink / raw)
To: 'Hiroshi Shimamoto', Skidmore, Donald C, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E08FF6@BPXM14GP.gisp.nec.co.jp>
From: Hiroshi Shimamoto
> My concern is what is the real issue that VF multicast promiscuous mode can cause.
> I think there is the 4k entries to filter multicast address, and the current ixgbe/ixgbevf
> can turn all bits on from VM. That is almost same as enabling multicast promiscuous mode.
> I mean that we can receive all multicast addresses by an onerous operation in untrusted VM.
> I think we should clarify what is real security issue in this context.
If you are worried about passing un-enabled multicasts to users then
what about doing a software hash of received multicasts and checking
against an actual list of multicasts enabled for that hash entry.
Under normal conditions there is likely to be only a single address to check.
It may (or may not) be best to use the same hash as any hashing hardware
filter uses.
David
^ permalink raw reply
* Re: [PATCH net-next] rhashtable: rhashtable_remove() must unlink in both tbl and future_tbl
From: Thomas Graf @ 2015-01-21 11:56 UTC (permalink / raw)
To: Ying Xue
Cc: davem, richard.alpe@ericsson.com >> Richard Alpe, Netdev,
tipc-discussion
In-Reply-To: <54BF65C2.9010404@windriver.com>
On 01/21/15 at 04:39pm, Ying Xue wrote:
> Your below changes are reasonable for me. But after I applied the patch,
> my reported two issues still happened :( And failure logs are completely
> same before.
Correct. The patch fixes the panic that you reported as well:
[ 17.033416] BUG: unable to handle kernel paging request at
ffffffffffffff77
[ 17.034009] IP: [<ffffffffa0019b8c>] jhash+0xec/0x160 [tipc]
[...]
[ 17.034009] Call Trace:
[ 17.034009] [<ffffffff813b38e9>] head_hashfn+0x29/0x50
[ 17.034009] [<ffffffff813b475b>] rhashtable_expand+0x37b/0x680
[ 17.034009] [<ffffffff813b4b38>] rht_deferred_worker+0xd8/0xe0
[ 17.034009] [<ffffffff81074d10>] process_one_work+0x200/0x800
[ 17.034009] [<ffffffff81074c71>] ? process_one_work+0x161/0x800
[ 17.034009] [<ffffffff81074c71>] ? process_one_work+0x161/0x800
[ 17.034009] [<ffffffff8107533c>] process_scheduled_works+0x2c/0x40
[ 17.034009] [<ffffffff810758d3>] worker_thread+0x253/0x4b0
[ 17.034009] [<ffffffff81075680>] ? rescuer_thread+0x330/0x330
[ 17.034009] [<ffffffff8107bb6f>] kthread+0xef/0x110
[ 17.034009] [<ffffffff8107ba80>] ? flush_kthread_work+0x170/0x170
[ 17.034009] [<ffffffff81a44a2c>] ret_from_fork+0x7c/0xb0
[ 17.034009] [<ffffffff8107ba80>] ? flush_kthread_work+0x170/0x170
^ permalink raw reply
* [PATCH net-next v2] rhashtable: rhashtable_remove() must unlink in both tbl and future_tbl
From: Thomas Graf @ 2015-01-21 11:54 UTC (permalink / raw)
To: davem, Sergei Shtylyov
Cc: Ying Xue, richard.alpe@ericsson.com >> Richard Alpe, Netdev,
tipc-discussion
In-Reply-To: <54BEA1F9.1050908@cogentembedded.com>
As removals can occur during resizes, entries may be referred to from
both tbl and future_tbl when the removal is requested. Therefore
rhashtable_remove() must unlink the entry in both tables if this is
the case. The existing code did search both tables but stopped when it
hit the first match.
Failing to unlink in both tables resulted in use after free.
Fixes: 97defe1ecf86 ("rhashtable: Per bucket locks & deferred expansion/shrinking")
Reported-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
v2: 12 digit commit reference, commit msg clarification as pointed
out by Sergei.
lib/rhashtable.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 84a78e3..bc2d0d8 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -585,6 +585,7 @@ bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj)
struct rhash_head *he;
spinlock_t *lock;
unsigned int hash;
+ bool ret = false;
rcu_read_lock();
tbl = rht_dereference_rcu(ht->tbl, ht);
@@ -602,17 +603,16 @@ restart:
}
rcu_assign_pointer(*pprev, obj->next);
- atomic_dec(&ht->nelems);
-
- spin_unlock_bh(lock);
-
- rhashtable_wakeup_worker(ht);
-
- rcu_read_unlock();
- return true;
+ ret = true;
+ break;
}
+ /* The entry may be linked in either 'tbl', 'future_tbl', or both.
+ * 'future_tbl' only exists for a short period of time during
+ * resizing. Thus traversing both is fine and the added cost is
+ * very rare.
+ */
if (tbl != rht_dereference_rcu(ht->future_tbl, ht)) {
spin_unlock_bh(lock);
@@ -625,9 +625,15 @@ restart:
}
spin_unlock_bh(lock);
+
+ if (ret) {
+ atomic_dec(&ht->nelems);
+ rhashtable_wakeup_worker(ht);
+ }
+
rcu_read_unlock();
- return false;
+ return ret;
}
EXPORT_SYMBOL_GPL(rhashtable_remove);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Wolfgang Grandegger @ 2015-01-21 11:53 UTC (permalink / raw)
To: Andri Yngvason
Cc: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Marc Kleine-Budde, Linux-CAN, netdev, LKML
In-Reply-To: <20150121103319.14511.57709@shannon>
On Wed, 21 Jan 2015 10:33:19 +0000, Andri Yngvason
<andri.yngvason@marel.com> wrote:
> Quoting Ahmed S. Darwish (2015-01-20 21:45:37)
>> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>
>> Replace most of the can interface's state and error counters
>> handling with the new can-dev can_change_state() mechanism.
>>
>> Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
>> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>> ---
>> drivers/net/can/usb/kvaser_usb.c | 114
>> +++++++++++++++++++--------------------
>> 1 file changed, 55 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/kvaser_usb.c
>> b/drivers/net/can/usb/kvaser_usb.c
>> index 971c5f9..0386d3f 100644
>> --- a/drivers/net/can/usb/kvaser_usb.c
>> +++ b/drivers/net/can/usb/kvaser_usb.c
...
>
> Looks good.
Would be nice to see some "candump" traces as well. Ahmed, could you
please generate such traces doing:
1. Execute in a session:
# candump -t d -e any,0:0,#FFFFFFFF
2. Execute in another session:
# cangen -g 10 -D i can0
3. Disconnect the CAN cable
4. After a while reconnect the CAN cable
5. Stop candump and save the trace.
and to test bus-off:
1. Execute in a session:
# candump -t d -e any,0:0,#FFFFFFFF
2. Execute in another session:
# cangen -g 10 -D i can0
3. Short-circuit the CAN low and high wires of the CAN cable
4. After a while remove the short-circuit.
5. Stop candump and save the trace.
Thanks,
Wolfgang.
^ permalink raw reply
* [PATCH v2] net: wireless: atmel: Remove open-coded and wrong strcasecmp
From: Rasmus Villemoes @ 2015-01-21 11:52 UTC (permalink / raw)
To: Simon Kelley, Kalle Valo
Cc: Sergei Shtylyov, Rasmus Villemoes, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1421414907-26764-1-git-send-email-linux@rasmusvillemoes.dk>
The kernel's string library does in fact have strcasecmp, at least
since ded220bd8f08 ("[STRING]: Move strcasecmp/strncasecmp to
lib/string.c"). Moreover, this open-coded version is in fact wrong: If
the strings only differ in their last character, a and b have already
been incremented to point to the terminating NUL bytes, so they would
wrongly be treated as equal.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v2: Address Sergei's comments to the commit message.
drivers/net/wireless/atmel.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/net/wireless/atmel.c b/drivers/net/wireless/atmel.c
index 9183f1cf89a7..55db9f03eb2a 100644
--- a/drivers/net/wireless/atmel.c
+++ b/drivers/net/wireless/atmel.c
@@ -45,7 +45,6 @@
#include <linux/ptrace.h>
#include <linux/slab.h>
#include <linux/string.h>
-#include <linux/ctype.h>
#include <linux/timer.h>
#include <asm/byteorder.h>
#include <asm/io.h>
@@ -2699,16 +2698,7 @@ static int atmel_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
domain[REGDOMAINSZ] = 0;
rc = -EINVAL;
for (i = 0; i < ARRAY_SIZE(channel_table); i++) {
- /* strcasecmp doesn't exist in the library */
- char *a = channel_table[i].name;
- char *b = domain;
- while (*a) {
- char c1 = *a++;
- char c2 = *b++;
- if (tolower(c1) != tolower(c2))
- break;
- }
- if (!*a && !*b) {
+ if (!strcasecmp(channel_table[i].name, domain)) {
priv->config_reg_domain = channel_table[i].reg_domain;
rc = 0;
}
--
2.1.3
^ permalink raw reply related
* [PATCH net] ipv6: tcp: fix race in IPV6_2292PKTOPTIONS
From: Eric Dumazet @ 2015-01-21 11:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
IPv6 TCP sockets store in np->pktoptions skbs, and use skb_set_owner_r()
to charge the skb to socket.
It means that destructor must be called while socket is locked.
Therefore, we cannot use skb_get() or atomic_inc(&skb->users)
to protect ourselves : kfree_skb() might race with other users
manipulating sk->sk_forward_alloc
Fix this race by holding socket lock for the duration of
ip6_datagram_recv_ctl()
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Since this bug is very old, feel free to apply on net-next ;)
net/ipv6/ipv6_sockglue.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 66980d8d98d1f5b3ef7a50dc33cb9b617f25604d..8d766d9100cba408525faf5818b7b0c6b6bc543c 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -996,13 +996,9 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
lock_sock(sk);
skb = np->pktoptions;
if (skb)
- atomic_inc(&skb->users);
- release_sock(sk);
-
- if (skb) {
ip6_datagram_recv_ctl(sk, &msg, skb);
- kfree_skb(skb);
- } else {
+ release_sock(sk);
+ if (!skb) {
if (np->rxopt.bits.rxinfo) {
struct in6_pktinfo src_info;
src_info.ipi6_ifindex = np->mcast_oif ? np->mcast_oif :
^ permalink raw reply related
* Re: [PATCH] rhashtable: fix rht_for_each_entry_safe() endless loop
From: Thomas Graf @ 2015-01-21 11:43 UTC (permalink / raw)
To: Patrick McHardy; +Cc: davem, netdev
In-Reply-To: <1421838733-16495-1-git-send-email-kaber@trash.net>
On 01/21/15 at 11:12am, Patrick McHardy wrote:
> "next" is not updated, causing an endless loop for buckets with more than
> one element.
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
Fixes: 88d6ed15acff ("rhashtable: Convert bucket iterators to take table and index")
Acked-by: Thomas Graf <tgraf@suug.ch>
Dave: This only affects net-next.
^ permalink raw reply
* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Hiroshi Shimamoto @ 2015-01-21 11:38 UTC (permalink / raw)
To: Skidmore, Donald C, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, Hayato Momma, linux-kernel@vger.kernel.org
In-Reply-To: <F6FB0E698C9B3143BDF729DF222866469129B844@ORSMSX110.amr.corp.intel.com>
> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
>
>
>
> > -----Original Message-----
> > From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> > Sent: Tuesday, January 20, 2015 5:07 PM
> > To: Skidmore, Donald C; Bjørn Mork
> > Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi, Sy
> > Jong; linux-kernel@vger.kernel.org; Hayato Momma
> > Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous
> > mode control
> >
> > > Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast
> > > promiscuous mode control
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Hiroshi Shimamoto [mailto:h-shimamoto@ct.jp.nec.com]
> > > > Sent: Tuesday, January 20, 2015 3:40 PM
> > > > To: Bjørn Mork
> > > > Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Choi,
> > > > Sy Jong; linux-kernel@vger.kernel.org; Hayato Momma
> > > > Subject: Re: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast
> > > > promiscuous mode control
> > > >
> > > > > Subject: Re: [PATCH 1/2] if_link: Add VF multicast promiscuous
> > > > > mode control
> > > > >
> > > > > Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> > > > >
> > > > > > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > > > > >
> > > > > > Add netlink directives and ndo entry to control VF multicast
> > > > > > promiscuous
> > > > mode.
> > > > > >
> > > > > > Intel ixgbe and ixgbevf driver can handle only 30 multicast MAC
> > > > > > addresses per VF. It means that we cannot assign over 30 IPv6
> > > > > > addresses to a single VF interface on VM. We want thousands IPv6
> > > > addresses in VM.
> > > > > >
> > > > > > There is capability of multicast promiscuous mode in Intel 82599 chip.
> > > > > > It enables all multicast packets are delivered to the target VF.
> > > > > >
> > > > > > This patch prepares to control that VF multicast promiscuous
> > > > functionality.
> > > > >
> > > > > Adding a new hook for this seems over-complicated to me. And it
> > > > > still doesn't solve the real problems that
> > > > > a) the user has to know about this limit, and
> > > > > b) manually configure the feature
> > > > >
> > > > > Most of us, lacking the ability to imagine such arbitrary hardware
> > > > > limitations, will go through a few hours of frustrating debugging
> > > > > before we figure this one out...
> > > > >
> > > > > Why can't the ixgbevf driver just automatically signal the ixgbe
> > > > > driver to enable multicast promiscuous mode whenever the list
> > > > > grows past the limit?
> > > >
> > > > I had submitted a patch to change ixgbe and ixgbevf driver for this issue.
> > > > https://lkml.org/lkml/2014/11/27/269
> > > >
> > > > The previous patch introduces API between ixgbe and ixgbevf driver
> > > > to enable multicast promiscuous mode, and ixgbevf enables it
> > > > automatically if the number of addresses is over than 30.
> > >
> > > I believe the issue is with allowing a VF to automatically enter
> > > Promiscuous Multicast without the PF's ok is concern
> >
> > So you mean that we should take care about enabling VF multicast
> > promiscuous mode in host side, right? The host allows multicast promiscuous
> > and VF requests it too, then enables VF multicast promiscuous mode.
> > So, what is preferred way to do in host do you think?
>
> I think we are saying the same thing. I believe it would be fine if the VF requests this to happen (threw a mailbox message
> like you set up) and the PF will do it if the systems policy has been set up that way (as you did with the control mode).
>
> This way the behavior (related to multicast) is the same as it has been, unless the system has been setup specifically
> to allow VF multicast promiscuous mode.
Now I understand what you're saying.
Will make patches that make knob in host/PF and ixgbe/ixgbevf interface.
I think I should try to find whether there is a way to make the know without ndo.
>
> I know it's been mentioned that this is onerous on those who want this behavior to be automatic, but I don't see how else
> it could be done and take account for people that are concerned about allowing a (possibly untrusted) VM promoting itself
> in to multicast promiscuous mode.
I didn't mind its onerousness so much.
My concern is what is the real issue that VF multicast promiscuous mode can cause.
I think there is the 4k entries to filter multicast address, and the current ixgbe/ixgbevf
can turn all bits on from VM. That is almost same as enabling multicast promiscuous mode.
I mean that we can receive all multicast addresses by an onerous operation in untrusted VM.
I think we should clarify what is real security issue in this context.
thanks,
Hiroshi
>
> >
> > > over VM isolation. Of course that isolation, when it comes to multicast, is
> > rather limited anyway given that our multicast
> > > filter uses only 12-bit of the address for a match. Still this (or
> > > doing it by default) would only open that up considerably more (all
> > > multicasts). I assume for your application you're not concerned, but are
> > there other use cases that would worry about such things?
> >
> > Sorry I couldn't catch the point.
> >
> > What is the issue? I think there is no difference for the users who don't want
> > many multicast addresses in guest. In the current implementation,
> > overflowed multicast addresses silently discarded in ixgbevf. I believe there
> > is no user who want to use over 30 multicast addresses now. If VF multicast
> > promiscuous mode is enabled in certain VF, the behavior of other VFs is not
> > changed.
> >
> > thanks,
> > Hiroshi
> >
> > >
> > > Thanks,
> > > -Don Skidmore <donald.c.skidmore@intel.com>
> > >
> > > >
> > > > I got some comment and I would like to clarify the point, but there
> > > > was no answer.
> > > > That's the reason I submitted this patch.
> > > >
> > > > Do you think a patch for the ixgbe/ixgbevf driver is preferred?
> > > >
> > > >
> > > > thanks,
> > > > Hiroshi
> > > >
> > > > >
> > > > > I'd also like to note that this comment in
> > > > > drivers/net/ethernet/intel/ixgbevf/vf.c
> > > > > indicates that the author had some ideas about how more than 30
> > > > > addresses could/should be handled:
> > > > >
> > > > > static s32 ixgbevf_update_mc_addr_list_vf(struct ixgbe_hw *hw,
> > > > > struct net_device *netdev)
> > > > > {
> > > > > struct netdev_hw_addr *ha;
> > > > > u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> > > > > u16 *vector_list = (u16 *)&msgbuf[1];
> > > > > u32 cnt, i;
> > > > >
> > > > > /* Each entry in the list uses 1 16 bit word. We have 30
> > > > > * 16 bit words available in our HW msg buffer (minus 1 for the
> > > > > * msg type). That's 30 hash values if we pack 'em right. If
> > > > > * there are more than 30 MC addresses to add then punt the
> > > > > * extras for now and then add code to handle more than 30 later.
> > > > > * It would be unusual for a server to request that many multi-cast
> > > > > * addresses except for in large enterprise network environments.
> > > > > */
> > > > >
> > > > >
> > > > >
> > > > > The last 2 lines of that comment are of course totally bogus and
> > > > > pointless and should be deleted in any case... It's obvious that
> > > > > 30 multicast addresses is ridiculously low for lots of normal use cases.
> > > > >
> > > > >
> > > > > Bjørn
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-21 11:37 UTC (permalink / raw)
To: Patrick McHardy
Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121103959.GR3012@acer.localdomain>
On 01/21/15 at 10:40am, Patrick McHardy wrote:
> On the danger of repeating myself, every (converted) user requires
> that we at least keep the existing semantics since it is exposed to
> userspace. My opinion is that NLM_F_DUMP_INTR is fine if userspace
> indicates support, but without that, we have to take care of that
> in the kernel.
Absolutely agreed. I think this is an excellent low cost path for
future users where dumps are rare.
> An automatic restart handles this well. Userspace always had to
> expect duplicates.
Maybe I don't understand the restart yet. How do you restart if the
dump was already started and the user has read part of the dump?
^ permalink raw reply
* [PATCH] rhashtable: fix rht_for_each_entry_safe() endless loop
From: Patrick McHardy @ 2015-01-21 11:12 UTC (permalink / raw)
To: tgraf; +Cc: davem, netdev, Patrick McHardy
"next" is not updated, causing an endless loop for buckets with more than
one element.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
include/linux/rhashtable.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 9570832..83944f9 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -260,7 +260,9 @@ void rhashtable_destroy(struct rhashtable *ht);
next = !rht_is_a_nulls(pos) ? \
rht_dereference_bucket(pos->next, tbl, hash) : NULL; \
(!rht_is_a_nulls(pos)) && rht_entry(tpos, pos, member); \
- pos = next)
+ pos = next, \
+ next = !rht_is_a_nulls(pos) ? \
+ rht_dereference_bucket(pos->next, tbl, hash) : NULL)
/**
* rht_for_each_rcu_continue - continue iterating over rcu hash chain
--
2.1.0
^ permalink raw reply related
* 2015 OFFER
From: CHEVROLET CAR COMPANY @ 2015-01-21 9:51 UTC (permalink / raw)
YOUR EMAIL ID HAVE WON 3 CRORES 35LAHKS INR & A CAR FROM CHEVROLET CAR
COMPANY. TO CLAIM SEND YOUR EMAIL ID/NAME/PHONE NO/AGE/ADDRESS/OCCUPATION
TO cvclaim2@gmail.com
^ permalink raw reply
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Andri Yngvason @ 2015-01-21 11:00 UTC (permalink / raw)
To: Marc Kleine-Budde, Ahmed S. Darwish, Olivier Sobrie,
Oliver Hartkopp, Wolfgang Grandegger
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <54BF8326.3080406@pengutronix.de>
Quoting Marc Kleine-Budde (2015-01-21 10:44:54)
> On 01/21/2015 11:33 AM, Andri Yngvason wrote:
> > Quoting Ahmed S. Darwish (2015-01-20 21:45:37)
> >> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> >>
> >> Replace most of the can interface's state and error counters
> >> handling with the new can-dev can_change_state() mechanism.
> >>
> >> Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
> >> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> >> ---
> >> drivers/net/can/usb/kvaser_usb.c | 114 +++++++++++++++++++--------------------
> >> 1 file changed, 55 insertions(+), 59 deletions(-)
> >>
> >> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> >> index 971c5f9..0386d3f 100644
> >> --- a/drivers/net/can/usb/kvaser_usb.c
> >> +++ b/drivers/net/can/usb/kvaser_usb.c
>
> > Looks good.
>
> Is this an Acked-by?
>
ACK.
--
Andri
^ permalink raw reply
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Marc Kleine-Budde @ 2015-01-21 10:44 UTC (permalink / raw)
To: Andri Yngvason, Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150121103319.14511.57709@shannon>
[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]
On 01/21/2015 11:33 AM, Andri Yngvason wrote:
> Quoting Ahmed S. Darwish (2015-01-20 21:45:37)
>> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>
>> Replace most of the can interface's state and error counters
>> handling with the new can-dev can_change_state() mechanism.
>>
>> Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
>> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>> ---
>> drivers/net/can/usb/kvaser_usb.c | 114 +++++++++++++++++++--------------------
>> 1 file changed, 55 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
>> index 971c5f9..0386d3f 100644
>> --- a/drivers/net/can/usb/kvaser_usb.c
>> +++ b/drivers/net/can/usb/kvaser_usb.c
> Looks good.
Is this an Acked-by?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Patrick McHardy @ 2015-01-21 10:40 UTC (permalink / raw)
To: Thomas Graf; +Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121103421.GR20315@casper.infradead.org>
On 21.01, Thomas Graf wrote:
> On 01/21/15 at 08:58pm, Herbert Xu wrote:
> > On Wed, Jan 21, 2015 at 09:49:28AM +0000, Thomas Graf wrote:
> > >
> > > An entry can move between different tables and thus chains need to be
> > > marked to identify what list a lookup ended up searching in. It's not
> > > the nulls marker itself that is needed, it's the bits in the last next
> > > pointer identifying the list that the nulls marker allows to be used
> > > which are essential.
> >
> > Can you describe in more detail how it's going to be used? I don't
> > see how I could use the bit if you need it to indicate the end of
> > the list.
>
> Another thought: Instead of storing that bit in the next pointer, we
> could require the user to store this bit, i.e. two new function
> pointers to rhashtable_params, set_walk_bit() and get_walk_bit(),
> which take the hashed object as argument and if a rhashtable user
> requires consistent dumps he can provide these functions to store
> the flag.
On the danger of repeating myself, every (converted) user requires
that we at least keep the existing semantics since it is exposed to
userspace. My opinion is that NLM_F_DUMP_INTR is fine if userspace
indicates support, but without that, we have to take care of that
in the kernel.
An automatic restart handles this well. Userspace always had to
expect duplicates.
^ permalink raw reply
* Re: [PATCH net] ipv4: try to cache dst_entries which would cause a redirect
From: Hannes Frederic Sowa @ 2015-01-21 10:39 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev, Marcelo Leitner, Florian Westphal
In-Reply-To: <alpine.LFD.2.11.1501211041360.2329@ja.home.ssi.bg>
Hi Julian,
On Mi, 2015-01-21 at 10:56 +0200, Julian Anastasov wrote:
> On Tue, 20 Jan 2015, Hannes Frederic Sowa wrote:
>
> > Not caching dst_entries which cause redirects could be exploited by hosts
> > on the same subnet, causing a severe DoS attack. This effect aggravated
> > since commit f88649721268999 ("ipv4: fix dst race in sk_dst_get()").
> >
> > Lookups causing redirects will be allocated with DST_NOCACHE set which
> > will force dst_release to free them via RCU. Unfortunately waiting for
> > RCU grace period just takes too long, we can end up with >1M dst_entries
> > waiting to be released and the system will run OOM. rcuos threads cannot
> > catch up under high softirq load.
> >
> > Attaching the flag to emit a redirect later on to the specific skb allows
> > us to cache those dst_entries thus reducing the pressure on allocation
> > and deallocation.
> >
> > This issue was discovered by Marcelo Leitner.
>
> Change looks good to me but additional place
> should be changed too: inet_rtm_getroute() will call
> ip_route_input() and later rt_fill_info() will put
> rt_flags in rtm_flags. We have to set RTCF_DOREDIRECT
> just in rtm_flags depending on IPSKB_DOREDIRECT becuase
> iproute needs to print "redirect". You can test it with
> ip route get ... iif INDEV
Very good catch, thanks. Will post v2 soon.
Thanks,
Hannes
^ permalink raw reply
* RE: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: David Laight @ 2015-01-21 10:36 UTC (permalink / raw)
To: 'Thomas Graf', Herbert Xu
Cc: Patrick McHardy, davem@davemloft.net, paulmck@linux.vnet.ibm.com,
ying.xue@windriver.com, netdev@vger.kernel.org,
netfilter-devel@vger.kernel.org
In-Reply-To: <20150121093722.GM20315@casper.infradead.org>
From: Thomas Graf
> On 01/21/15 at 04:08pm, Herbert Xu wrote:
...
> > Essentially I need a bit to indicate an entry in the bucket chain
> > should be skipped, either because it has just been removed or that
> > it is a walker entry (see xfrm_state_walk).
> >
> > The way it'll work then is exactly the same as xfrm_state_walk,
> > except that the linked list is broken up into individual buckets.
> >
> > Of course we'll still need to postpone resizes (and rehashes which
> > is what my work is about) during a walk but I think that's a fair
> > price to pay.
>
> If I understand this correctly we also need to block out parallel
> walkers and we need to start taking bucket locks while walking to
> modify the walker mark bit in peace.
Running 'walker | grep foo | less' really shouldn't stop any activity,
including further requests by the same person to dump the same table in a
different window.
I would also expect the above request to be able to continue correctly
when restarted hours later (unless something unusual - like a resize -
happens).
Suppressing 'table shrink' because you think a walker might be active
if probably a good idea, and just requires a timestamp of the last
walk action.
Thought...
It is possible to break walking only on hash chain boundaries?
At least in the case where the response buffer is large enough
for a full 'chain' of entries.
Also what happens if there is a page fault on any userpage?
You don't want to be holding a mutex.
David
^ 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