* Re: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Pravin Shelar @ 2015-01-21 9:08 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, Linux Netdev List
In-Reply-To: <CA+mtBx90TipY9uJvW2uQfrCYL9JNM9Rk75HzdJPhE0qsdy8dnw@mail.gmail.com>
On Tue, Jan 20, 2015 at 3:06 PM, Tom Herbert <therbert@google.com> wrote:
> On Tue, Jan 20, 2015 at 12:25 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
>> Following patch series adds support for Stateless Transport
>> Tunneling protocol.
>> STT uses TCP segmentation offload available in most of NIC. On
>> packet xmit STT driver appends STT header along with TCP header
>> to the packet. For GSO packet GSO parameters are set according
>> to tunnel configuration and packet is handed over to networking
>> stack. This allows use of segmentation offload available in NICs
>>
>> Netperf unidirectional test gives ~9.4 Gbits/s performance on 10Gbit
>> NIC with 1500 byte MTU with two TCP streams.
>>
> Having packets marked TCP which really aren't TCP is a rather scary
> prospect to deploy in a real data center (TCP is kind of an important
> protocol ;-) ). Can you give some more motivation on this, more data
> that shows what the benefits are and how this compares to equivalent
> encapsulation protocols that implement GRO and GSO.
>
There are multi-year deployments of STT, So it is already in real data center.
Biggest advantage is STT does not need new NIC with tunnel offload.
Any NIC that supports TCP offload can be used to achieve better
performance.
Following are numbers you asked for.
Setup: net-next branch on server and client.
netperf: TCP unidirectional tests with 5 streams. Numbers are averaged
over 3 runs of 50 sec.
VXLAN:
CPU
Client: 1.6
Server: 14.2
Throughput: 5.6 Gbit/s
VXLAN with rcsum:
CPU
Client: 0.89
Server: 12.4
Throughput: 5.8 Gbit/s
STT:
CPU
Client: 1.28
Server: 4.0
Throughput: 9.5 Gbit/s
Thanks,
Pravin.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Herbert Xu @ 2015-01-21 9:14 UTC (permalink / raw)
To: Thomas Graf
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121051555.GA23300@gondor.apana.org.au>
On Wed, Jan 21, 2015 at 04:15:55PM +1100, Herbert Xu wrote:
> On Wed, Jan 21, 2015 at 04:08:19PM +1100, Herbert Xu wrote:
> >
> > OK I think I have a solution for you guys. But first you'll need to
> > wait for me to undo the nulls stuff so I can steal that bit which
> > is central to my solution.
> >
> > 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.
>
> I failed to address the security aspect of this approach. Obviously
> this can only work if the only entites doing the walk are trusted.
> Which means that the vast majority (if not all) hash table users
> would be excluded, in particular, netlink.
OK maybe we can get around this. So we will postpone the resize
or rehash when a walk is in place, however, when we hit a hard
limit, i.e., when insert would otherwise fail, then we do the
rehash regardless of any outstanding walks. The walks will just
behave erratically or we could force them to start from scratch
again.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* [patch] bridge: simplify br_getlink() a bit
From: Dan Carpenter @ 2015-01-21 9:22 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, bridge, kernel-janitors, David S. Miller
Static checkers complain that we should maybe set "ret" before we do the
"goto out;". They interpret the NULL return from br_port_get_rtnl() as
a failure and forgetting to set the error code is a common bug in this
situation.
The code is confusing but it's actually correct. We are returning zero
deliberately. Let's re-write it a bit to be more clear.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 528cf27..3875ea51 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -311,17 +311,14 @@ errout:
int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
struct net_device *dev, u32 filter_mask)
{
- int err = 0;
struct net_bridge_port *port = br_port_get_rtnl(dev);
if (!port && !(filter_mask & RTEXT_FILTER_BRVLAN) &&
!(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED))
- goto out;
+ return 0;
- err = br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, NLM_F_MULTI,
- filter_mask, dev);
-out:
- return err;
+ return br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, NLM_F_MULTI,
+ filter_mask, dev);
}
static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
^ permalink raw reply related
* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Bjørn Mork @ 2015-01-21 9:26 UTC (permalink / raw)
To: Hiroshi Shimamoto
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E07B7C@BPXM14GP.gisp.nec.co.jp>
Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
>> 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 got some comment and I would like to clarify the point, but there was no
> answer.
> That's the reason I submitted this patch.
Thanks. Yes, now I understand why you want to have a policy knob.
I still think the policy could select between "automatic"/"disallowed"
instead of "enabled"/"disabled", but that's a minor detail. Likewise is
the actual implemention of "automatic". I think you could do that
within the current VF-PF protocol by overloading the MC address "count".
But a more generic question for netdev is: Does this VF policy API
really scale?
How many different VF policy tunables can you imaging add up over a few
years and drivers. Currently each policy flag require its own ndo hook.
I probably don't have much to say here, but IMHO this scheme had already
failed when .ndo_set_vf_spoofchk was added..
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 9:37 UTC (permalink / raw)
To: Herbert Xu
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121050819.GA23062@gondor.apana.org.au>
On 01/21/15 at 04:08pm, Herbert Xu wrote:
> On Tue, Jan 20, 2015 at 03:35:56PM +0000, Thomas Graf wrote:
> > On 01/20/15 at 03:21pm, Patrick McHardy wrote:
> > > I think its preferrable to make the need to handle NETLINK_F_DUMP_INTR
> > > as noticable as possible and not hide it. Silent failure is the worst
> > > kind of failure.
> >
> > I agree to that. The point here is to avoid unnecessary use of
> > NETLINK_F_DUMP_INTR if all entries fit into a single message buffer.
>
> OK I think I have a solution for you guys. But first you'll need to
> wait for me to undo the nulls stuff so I can steal that bit which
> is central to my solution.
Without having seen your code, can we make it configurable on what
the bit is used for? Use of nulls marker is a strict requirement for
some targeted users of rhashtable.
> 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.
> This also means handling insertion failures but I think that
> should be acceptable if we make it based on a configurable maximum
> chain length along with forced resize/rehash where possible.
>
> Note that this can be made optional, i.e., if the user can afford
> memory to do their own walking (e.g., xfrm_state), then none of
> this needs to happen and it'll just work as it does now. IOW if
> you don't use this special rhashtable walk function then you're
> not affected.
That sounds like the best option to me.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Herbert Xu @ 2015-01-21 9:38 UTC (permalink / raw)
To: Thomas Graf
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121093722.GM20315@casper.infradead.org>
On Wed, Jan 21, 2015 at 09:37:22AM +0000, Thomas Graf wrote:
>
> Without having seen your code, can we make it configurable on what
> the bit is used for? Use of nulls marker is a strict requirement for
> some targeted users of rhashtable.
What do they need this for?
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-21 9:49 UTC (permalink / raw)
To: Herbert Xu
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121093836.GA25489@gondor.apana.org.au>
On 01/21/15 at 08:38pm, Herbert Xu wrote:
> On Wed, Jan 21, 2015 at 09:37:22AM +0000, Thomas Graf wrote:
> >
> > Without having seen your code, can we make it configurable on what
> > the bit is used for? Use of nulls marker is a strict requirement for
> > some targeted users of rhashtable.
>
> What do they need this for?
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.
This is on my plate next. Most of the work in rhashtable was done in
preparation of translating the TCP established table over to
rhashtable.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-21 9:56 UTC (permalink / raw)
To: Herbert Xu
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121091457.GA25289@gondor.apana.org.au>
On 01/21/15 at 08:14pm, Herbert Xu wrote:
> On Wed, Jan 21, 2015 at 04:15:55PM +1100, Herbert Xu wrote:
> > I failed to address the security aspect of this approach. Obviously
> > this can only work if the only entites doing the walk are trusted.
> > Which means that the vast majority (if not all) hash table users
> > would be excluded, in particular, netlink.
>
> OK maybe we can get around this. So we will postpone the resize
> or rehash when a walk is in place, however, when we hit a hard
> limit, i.e., when insert would otherwise fail, then we do the
> rehash regardless of any outstanding walks. The walks will just
> behave erratically or we could force them to start from scratch
> again.
Exactly. I think we also need a timer to abort walks because if
only a single walker is allowed, an attacker could start a walk and
not complete it to block out everybody else.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Herbert Xu @ 2015-01-21 9:58 UTC (permalink / raw)
To: Thomas Graf
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121094928.GN20315@casper.infradead.org>
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.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Herbert Xu @ 2015-01-21 9:59 UTC (permalink / raw)
To: Thomas Graf
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121095634.GO20315@casper.infradead.org>
On Wed, Jan 21, 2015 at 09:56:34AM +0000, Thomas Graf wrote:
>
> Exactly. I think we also need a timer to abort walks because if
> only a single walker is allowed, an attacker could start a walk and
> not complete it to block out everybody else.
My scheme should support an arbitrary number of walks. See how
xfrm_state_walk is implemented.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Patrick McHardy @ 2015-01-21 10:00 UTC (permalink / raw)
To: Thomas Graf; +Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121095634.GO20315@casper.infradead.org>
On 21.01, Thomas Graf wrote:
> On 01/21/15 at 08:14pm, Herbert Xu wrote:
> > On Wed, Jan 21, 2015 at 04:15:55PM +1100, Herbert Xu wrote:
> > > I failed to address the security aspect of this approach. Obviously
> > > this can only work if the only entites doing the walk are trusted.
> > > Which means that the vast majority (if not all) hash table users
> > > would be excluded, in particular, netlink.
> >
> > OK maybe we can get around this. So we will postpone the resize
> > or rehash when a walk is in place, however, when we hit a hard
> > limit, i.e., when insert would otherwise fail, then we do the
> > rehash regardless of any outstanding walks. The walks will just
> > behave erratically or we could force them to start from scratch
> > again.
>
> Exactly. I think we also need a timer to abort walks because if
> only a single walker is allowed, an attacker could start a walk and
> not complete it to block out everybody else.
Restarting them automatically sounds reasonable, duplicate entries have
always been possible. That makes me think, we could have userspace
indicate support for NLM_F_DUMP_INTR and otherwise always restart.
That would solve the problem very easily.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-21 10:23 UTC (permalink / raw)
To: Herbert Xu
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121095837.GA25750@gondor.apana.org.au>
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.
The usage will be identical to how __inet_lookup_listener() uses it.
If at the end of the lookup, we ended up in a different table than
we started, the lookup is restarted as an entry has moved to another
table while we were moving over it.
^ permalink raw reply
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Andri Yngvason @ 2015-01-21 10:33 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Marc Kleine-Budde
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150120214537.GB16828@linux>
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
> @@ -620,40 +620,43 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> }
>
> static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
> - const struct kvaser_usb_error_summary *es)
> + const struct kvaser_usb_error_summary *es,
> + struct can_frame *cf)
> {
> struct net_device_stats *stats;
> - enum can_state new_state;
> -
> - stats = &priv->netdev->stats;
> - new_state = priv->can.state;
> + enum can_state cur_state, new_state, tx_state, rx_state;
>
> netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
>
> - if (es->status & M16C_STATE_BUS_OFF) {
> - priv->can.can_stats.bus_off++;
> + stats = &priv->netdev->stats;
> + new_state = cur_state = priv->can.state;
> +
> + if (es->status & M16C_STATE_BUS_OFF)
> new_state = CAN_STATE_BUS_OFF;
> - } else if (es->status & M16C_STATE_BUS_PASSIVE) {
> - if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
> - priv->can.can_stats.error_passive++;
> + else if (es->status & M16C_STATE_BUS_PASSIVE)
> new_state = CAN_STATE_ERROR_PASSIVE;
> - }
>
> if (es->status == M16C_STATE_BUS_ERROR) {
> - if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> - ((es->txerr >= 96) || (es->rxerr >= 96))) {
> - priv->can.can_stats.error_warning++;
> + if ((cur_state < CAN_STATE_ERROR_WARNING) &&
> + ((es->txerr >= 96) || (es->rxerr >= 96)))
> new_state = CAN_STATE_ERROR_WARNING;
> - } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
> + else if (cur_state > CAN_STATE_ERROR_ACTIVE)
> new_state = CAN_STATE_ERROR_ACTIVE;
> - }
> }
>
> if (!es->status)
> new_state = CAN_STATE_ERROR_ACTIVE;
>
> + if (new_state != cur_state) {
> + tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
> + rx_state = (es->txerr <= es->rxerr) ? new_state : 0;
> +
> + can_change_state(priv->netdev, cf, tx_state, rx_state);
> + new_state = priv->can.state;
> + }
> +
> if (priv->can.restart_ms &&
> - (priv->can.state >= CAN_STATE_BUS_OFF) &&
> + (cur_state >= CAN_STATE_BUS_OFF) &&
> (new_state < CAN_STATE_BUS_OFF)) {
> priv->can.can_stats.restarts++;
> }
> @@ -665,18 +668,17 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
>
> priv->bec.txerr = es->txerr;
> priv->bec.rxerr = es->rxerr;
> - priv->can.state = new_state;
> }
>
> static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> const struct kvaser_msg *msg)
> {
> - struct can_frame *cf;
> + struct can_frame *cf, tmp_cf = { .can_id = CAN_ERR_FLAG, .can_dlc = CAN_ERR_DLC };
> struct sk_buff *skb;
> struct net_device_stats *stats;
> struct kvaser_usb_net_priv *priv;
> struct kvaser_usb_error_summary es = { };
> - enum can_state old_state;
> + enum can_state old_state, new_state;
>
> switch (msg->id) {
> case CMD_CAN_ERROR_EVENT:
> @@ -721,60 +723,54 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> }
>
> /* Update all of the can interface's state and error counters before
> - * trying any skb allocation that can actually fail with -ENOMEM.
> + * trying any memory allocation that can actually fail with -ENOMEM.
> + *
> + * We send a temporary stack-allocated error can frame to
> + * can_change_state() for the very same reason.
> + *
> + * TODO: Split can_change_state() responsibility between updating the
> + * can interface's state and counters, and the setting up of can error
> + * frame ID and data to userspace. Remove stack allocation afterwards.
> */
> old_state = priv->can.state;
> - kvaser_usb_rx_error_update_can_state(priv, &es);
> + kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
> + new_state = priv->can.state;
>
> skb = alloc_can_err_skb(priv->netdev, &cf);
> if (!skb) {
> stats->rx_dropped++;
> return;
> }
> + memcpy(cf, &tmp_cf, sizeof(*cf));
>
> - if (es.status & M16C_STATE_BUS_OFF) {
> - cf->can_id |= CAN_ERR_BUSOFF;
> -
> - if (!priv->can.restart_ms)
> - kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> - netif_carrier_off(priv->netdev);
> - } else if (es.status & M16C_STATE_BUS_PASSIVE) {
> - if (old_state != CAN_STATE_ERROR_PASSIVE) {
> - cf->can_id |= CAN_ERR_CRTL;
> -
> - if (es.txerr || es.rxerr)
> - cf->data[1] = (es.txerr > es.rxerr)
> - ? CAN_ERR_CRTL_TX_PASSIVE
> - : CAN_ERR_CRTL_RX_PASSIVE;
> - else
> - cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
> - CAN_ERR_CRTL_RX_PASSIVE;
> + if (new_state != old_state) {
> + if (es.status & M16C_STATE_BUS_OFF) {
> + if (!priv->can.restart_ms)
> + kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> + netif_carrier_off(priv->netdev);
> + }
> +
> + if (es.status == M16C_STATE_BUS_ERROR) {
> + if ((old_state >= CAN_STATE_ERROR_WARNING) ||
> + (es.txerr < 96 && es.rxerr < 96)) {
> + if (old_state > CAN_STATE_ERROR_ACTIVE) {
> + cf->can_id |= CAN_ERR_PROT;
> + cf->data[2] = CAN_ERR_PROT_ACTIVE;
> + }
> + }
> }
> - }
>
> - if (es.status == M16C_STATE_BUS_ERROR) {
> - if ((old_state < CAN_STATE_ERROR_WARNING) &&
> - ((es.txerr >= 96) || (es.rxerr >= 96))) {
> - cf->can_id |= CAN_ERR_CRTL;
> - cf->data[1] = (es.txerr > es.rxerr)
> - ? CAN_ERR_CRTL_TX_WARNING
> - : CAN_ERR_CRTL_RX_WARNING;
> - } else if (old_state > CAN_STATE_ERROR_ACTIVE) {
> + if (!es.status) {
> cf->can_id |= CAN_ERR_PROT;
> cf->data[2] = CAN_ERR_PROT_ACTIVE;
> }
> - }
>
> - if (!es.status) {
> - cf->can_id |= CAN_ERR_PROT;
> - cf->data[2] = CAN_ERR_PROT_ACTIVE;
> - }
> -
> - if (priv->can.restart_ms &&
> - (old_state >= CAN_STATE_BUS_OFF) &&
> - (priv->can.state < CAN_STATE_BUS_OFF)) {
> - cf->can_id |= CAN_ERR_RESTARTED;
> - netif_carrier_on(priv->netdev);
> + if (priv->can.restart_ms &&
> + (old_state >= CAN_STATE_BUS_OFF) &&
> + (new_state < CAN_STATE_BUS_OFF)) {
> + cf->can_id |= CAN_ERR_RESTARTED;
> + netif_carrier_on(priv->netdev);
> + }
> }
>
> if (es.error_factor) {
> --
> 1.9.1
Looks good.
--
Andri
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-21 10:34 UTC (permalink / raw)
To: Herbert Xu
Cc: Patrick McHardy, davem, paulmck, ying.xue, netdev,
netfilter-devel
In-Reply-To: <20150121095837.GA25750@gondor.apana.org.au>
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.
^ 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
* 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: 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 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 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
* 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
* [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
* 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
* 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] 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
* [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
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