* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Hannes Frederic Sowa @ 2013-10-10 2:04 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Eric Dumazet, Josh Triplett, linux-kernel, mingo, laijs, dipankar,
akpm, mathieu.desnoyers, niv, tglx, peterz, rostedt, dhowells,
edumazet, darren, fweisbec, sbw, David S. Miller,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev
In-Reply-To: <20131010002833.GJ5790@linux.vnet.ibm.com>
On Wed, Oct 09, 2013 at 05:28:33PM -0700, Paul E. McKenney wrote:
> On Wed, Oct 09, 2013 at 05:12:40PM -0700, Eric Dumazet wrote:
> > On Wed, 2013-10-09 at 16:40 -0700, Josh Triplett wrote:
> >
> > > that. Constructs like list_del_rcu are much clearer, and not
> > > open-coded. Open-coding synchronization code is almost always a Bad
> > > Idea.
> >
> > OK, so you think there is synchronization code.
> >
> > I will shut up then, no need to waste time.
>
> As you said earlier, we should at least get rid of the memory barrier
> as long as we are changing the code.
Interesting thread!
Sorry to chime in and asking a question:
Why do we need an ACCESS_ONCE here if rcu_assign_pointer can do without one?
In other words I wonder why rcu_assign_pointer is not a static inline function
to use the sequence point in argument evaluation (if I remember correctly this
also holds for inline functions) to not allow something like this:
E.g. we want to publish which lock to take first to prevent an ABBA problem
(extreme example):
rcu_assign_pointer(lockptr, min(lptr1, lptr2));
Couldn't a compiler spill the lockptr memory location as a temporary buffer
if the compiler is under register pressure? (yes, this seems unlikely if we
flushed out most registers to memory because of the barrier, but still... ;) )
This seems to be also the case if we publish a multi-dereferencing pointers
e.g. ptr->ptr->ptr.
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: fix vport-netdev unregister
From: Jesse Gross @ 2013-10-10 1:24 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Pravin B Shelar, Jiri Pirko, dev@openvswitch.org,
netdev
In-Reply-To: <1381288051-3607-1-git-send-email-ast@plumgrid.com>
On Tue, Oct 8, 2013 at 8:07 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> The combination of two commits
>
> commit 8e4e1713e4
> ("openvswitch: Simplify datapath locking.")
>
> and
>
> commit 2537b4dd0a
> ("openvswitch:: link upper device for port devices")
>
> introduced a bug where upper_dev wasn't unlinked upon
> netdev_unregister notification
>
> The following steps:
>
> modprobe openvswitch
> ovs-dpctl add-dp test
> ip tuntap add dev tap1 mode tap
> ovs-dpctl add-if test tap1
> ip tuntap del dev tap1 mode tap
>
> are causing multiple warnings:
[...]
Hmm, you are right. Pravin mentioned some concerns about layering with
this patch so I believe he's trying to think of a way to minimize
that.
^ permalink raw reply
* Re: [PATCH v2.43 0/5] MPLS actions and matches
From: Simon Horman @ 2013-10-10 0:31 UTC (permalink / raw)
To: dev, netdev, Jesse Gross, Ben Pfaff
Cc: Pravin B Shelar, Ravi K, Isaku Yamahata, Joe Stringer
In-Reply-To: <1381132847-12589-1-git-send-email-horms@verge.net.au>
On Mon, Oct 07, 2013 at 05:00:42PM +0900, Simon Horman wrote:
> Hi,
>
> This series implements MPLS actions and matches based on work by
> Ravi K, Leo Alterman, Yamahata-san and Joe Stringer.
>
> This series provides two changes
>
> * Patches 1 - 3
>
> Provide user-space support for the VLAN/MPLS tag insertion order
> up to and including OpenFlow 1.2, and the different ordering
> specified from OpenFlow 1.3. In a nutshell the datapath always
> uses the OpenFlow 1.3 ordering, which is to always insert tags
> immediately after the L2 header, regardless of the presence of other
> tags. And ovs-vswtichd provides compatibility for the behaviour up
> to OpenFlow 1.2, which is that MPLS tags should follow VLAN tags
> if present.
>
> These patches have been updated since v2.42.
>
> Ben, these are for you to review.
>
> * Patches 4 and 5
>
> Adding basic MPLS action and match support to the kernel datapath
>
> These patches were last updated in v2.41.
>
> Jesse, these are for you to review.
Hi Jesse, Pravin and Ben,
I believe this series addresses the feedback that each of you
gave with regards to recent previous postings of this patch-set.
I'm wondering if you could find some time to review it.
>
>
> Differences between v2.43 and v2.42:
>
> * As suggested by Ben Pfaff
> Move vlan state into struct xlate_ctx
> 1. Add final_vlan_tci member to struct xlate_ctx instead of vlan_tci member
> struct xlate_xin. This seems to be a better palace for it as it does
> not need to be accessible from the caller.
> 2. Move local vlan_tci variable of do_xlate_actions() to be the
> next_vlan_tci member of strict xlate_ctx. This allows for it to work
> across resubmit actions and goto table instructions.
> * Code contributed by Ben Pfaff
> + Use enum for to control order of MPLS LSE insertion
> This makes the logic somewhat clearer
> * Add a helper push_mpls_from_openflow() to consolidate
> the same logic that appears in three locations.
>
>
> Differences between v2.42 and v2.41:
>
> * Rebase for:
> + 0585f7a ("datapath: Simplify mega-flow APIs.")
> + a097c0b ("datapath: Restructure datapath.c and flow.c")
> * As suggested by Jesse Gross
> + Take into account that push_mpls() will have freed the skb on error
> + Remove dubious !eth_p_mpls(skb->protocol) condition from push_mpls
> The !eth_p_mpls(skb->protocol) condition on setting inner_protocol
> has no effect. Its motivation was to ensure that inner_protocol was
> only set the first time that mpls_push occured. However this is already
> ensured by the !ovs_skb_get_inner_protocol(skb) condition.
> + Return -EINVAL instead of -ENOMEM from pop_mpls() if the skb is too short
> + Do not add @inner_protocol to kernel doc for struct ovs_skb_cb.
> The patch no longer adds an inner_protocol member to struct ovs_skb_cb
> + Do not add and set otherwise unsued inner_protocol variable in
> rpl_dev_queue_xmit()
> * As suggested by Pravin Shelar
> + Implement compatibility code in existing rpl_skb_gso_segment
> rather than introducing to use rpl___skb_gso_segment
>
>
> Differences between v2.41 and v2.40:
>
> * As suggested by Ben Pfaff
> + Expand struct ofpact_reg_load to include a mpls_before_vlan field
> rather than using the compat field of the ofpact field of
> struct ofpact_reg_load.
> + Pass version to ofpacts_pull_openflow11_actions and
> ofpacts_pull_openflow11_instructions. This removes the invalid
> assumption that that these functions are passed a full message and are
> thus able to deduce the OpenFlow version.
>
>
> Differences between v2.40 and v2.39:
>
> * Rebase for:
> + New dev_queue_xmit compat code
> + Updated put_vlan()
> + Removal of mpls_depth field from struct flow
> * As suggested by Jesse Gross
> + Remove bogus mac_len update from push_mpls()
> + Slightly simplify push_mpls() by using eth_hdr()
> + Remove dubious condition !eth_p_mpls(inner_protocol) on
> an skb being considered to be MPLS in netdev_send()
> + Only use compatibility code for MPLS GSO segmentation on kernels
> older than 3.11
> + Revamp setting of inner_protocol
> 1. Do not unconditionally set inner_protocol to the value of
> skb->protocol in ovs_execute_actions().
> 2. Initialise inner_protocol it to zero only if compatibility code is in
> use. In the case where compatibility code is not in use it will either
> be zero due since the allocation of the skb or some other value set
> by some other user.
> 3. Conditionally set the inner_protocol in push_mpls() to the value of
> skb->protocol when entering push_mpls(). The condition is that
> inner_protocol is zero and the value of skb->protocol is not an MPLS
> ethernet type.
> - This new scheme:
> + Pushes logic to set inner_protocol closer to the case where it is
> needed.
> + Avoids over-writing values set by other users.
> * As suggested by Pravin Shelar
> + Only set and restore skb->protocol in rpl___skb_gso_segment() in the
> case of MPLS
> + Add inner_protocol field to struct ovs_gso_cb instead of ovs_skb_cb.
> This moves compatibility code closer to where it is used
> and creates fewer differences with mainline.
> * Update comment on mac_len updates in datapath/actions.c
> * Remove HAVE_INNER_PROCOTOL and instead just check
> against kernel version 3.11 directly.
> HAVE_INNER_PROCOTOL is a hang-over from work done prior
> to the merge of inner_protocol into the kernel.
> * Remove dubious condition !eth_p_mpls(inner_protocol) on
> using inner_protocol as the type in rpl_skb_network_protocol()
> * Do not update type of features in rpl_dev_queue_xmit.
> Though arguably correct this is not an inherent part of
> the changes made by this patch.
> * Use skb_cow_head() in push_mpls()
> + Call skb_cow_head(skb, MPLS_HLEN) instead of
> make_writable(skb, skb->mac_len) to ensure that there is enough head
> room to push an MPLS LSE regardless of whether the skb is cloned or not.
> + This is consistent with the behaviour of rpl__vlan_put_tag().
> + This is a fix for crashes reported when performing mpls_push
> with headroom less than 4. This problem was introduced in v3.36.
> * Skip popping in mpls_pop if the skb is too short to contain an MPLS LSE
>
>
> Differences between v2.39 and v2.38:
>
> * Rebase for removal of vlan, checksum and skb->mark compat code
> - This includes adding adding a new patch,
> "[PATCH v2.39 6/7] datapath: Break out deacceleration portion of
> vlan_push" to allow re-use of some existing code.
>
>
> Differences between v2.38 and v2.37:
>
> * Rebase for SCTP support
> * Refactor validate_tp_port() to iterate over eth_types rather
> than open-coding the loop. With the addition of SCTP this logic
> is now used three times.
>
>
> Differences between v2.37 and v2.36:
>
> * Rebase
>
>
> Differences between v2.36 and v2.35:
>
> * Rebase
>
> * Do not add set_ethertype() to datapath/actions.c.
> As this patch has evolved this function had devolved into
> to sets of functionality wrapped into a single function with
> only one line of common code. Refactor things to simply
> open-code setting the ether type in the two locations where
> set_ethertype() was previously used. The aim here is to improve
> readability.
>
> * Update setting skb->ethertype after mpls push and pop.
> - In the case of push_mpls it should be set unconditionally
> as in v2.35 the behaviour of this function to always push
> an MPLS LSE before any VLAN tags.
> - In the case of mpls_pop eth_p_mpls(skb->protocol) is a better
> test than skb->protocol != htons(ETH_P_8021Q) as it will give the
> correct behaviour in the presence of other VLAN ethernet types,
> for example 0x88a8 which is used by 802.1ad. Moreover, it seems
> correct to update the ethernet type if it was previously set
> according to the top-most MPLS LSE.
>
> * Deaccelerate VLANs when pushing MPLS tags the
> - Since v2.35 MPLS push will insert an MPLS LSE before any VLAN tags.
> This means that if an accelerated tag is present it should be
> deaccelerated to ensure it ends up in the correct position.
>
> * Update skb->mac_len in push_mpls() so that it will be correct
> when used by a subsequent call to pop_mpls().
>
> As things stand I do not believe this is strictly necessary as
> ovs-vswitchd will not send a pop MPLS action after a push MPLS action.
> However, I have added this in order to code more defensively as I believe
> that if such a sequence did occur it would be rather unobvious why
> it didn't work.
>
> * Do not add skb_cow_head() call in push_mpls().
> It is unnecessary as there is a make_writable() call.
> This change was also made in v2.30 but some how the
> code regressed between then and v2.35.
>
>
> Differences between v2.35 and v2.34:
>
> * Add support for the tag ordering specified up until OpenFlow 1.2 and
> the ordering specified from OpenFlow 1.3.
>
> * Correct error in datapath patch's handling of GSO in the presence
> of MPLS and absence of VLANs.
>
>
> To aid review this series is available in git at:
>
> git://github.com/horms/openvswitch.git devel/mpls-v2.43
>
>
> Patch list and overall diffstat:
>
> Joe Stringer (3):
> odp: Allow VLAN actions after MPLS actions
> ofp-actions: Add separate OpenFlow 1.3 action parser
> lib: Support pushing of MPLS LSE before or after VLAN tag
>
> Simon Horman (2):
> datapath: Break out deacceleration portion of vlan_push
> datapath: Add basic MPLS support to kernel
>
> datapath/Modules.mk | 1 +
> datapath/actions.c | 158 ++++++++-
> datapath/datapath.c | 4 +-
> datapath/flow.c | 29 ++
> datapath/flow.h | 17 +-
> datapath/flow_netlink.c | 286 +++++++++++++--
> datapath/flow_netlink.h | 2 +-
> datapath/linux/compat/gso.c | 70 +++-
> datapath/linux/compat/gso.h | 41 +++
> datapath/linux/compat/include/linux/netdevice.h | 6 +-
> datapath/linux/compat/netdevice.c | 10 +-
> datapath/mpls.h | 15 +
> include/linux/openvswitch.h | 7 +-
> lib/flow.c | 2 +-
> lib/odp-util.c | 9 +-
> lib/odp-util.h | 2 +-
> lib/ofp-actions.c | 91 ++++-
> lib/ofp-actions.h | 16 +
> lib/ofp-print.c | 2 +-
> lib/ofp-util.c | 24 +-
> lib/ofp-util.h | 2 +-
> lib/packets.c | 10 +-
> lib/packets.h | 2 +-
> ofproto/ofproto-dpif-xlate.c | 132 +++++--
> tests/ofproto-dpif.at | 446 ++++++++++++++++++++++++
> utilities/ovs-ofctl.c | 8 +-
> 26 files changed, 1272 insertions(+), 120 deletions(-)
> create mode 100644 datapath/mpls.h
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-10 0:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: Josh Triplett, linux-kernel, mingo, laijs, dipankar, akpm,
mathieu.desnoyers, niv, tglx, peterz, rostedt, dhowells, edumazet,
darren, fweisbec, sbw, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <1381363960.4971.55.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, Oct 09, 2013 at 05:12:40PM -0700, Eric Dumazet wrote:
> On Wed, 2013-10-09 at 16:40 -0700, Josh Triplett wrote:
>
> > that. Constructs like list_del_rcu are much clearer, and not
> > open-coded. Open-coding synchronization code is almost always a Bad
> > Idea.
>
> OK, so you think there is synchronization code.
>
> I will shut up then, no need to waste time.
As you said earlier, we should at least get rid of the memory barrier
as long as we are changing the code.
Josh, what would you suggest as the best way to avoid the memory barrier,
keep sparse happy, and not be too ugly?
Thanx, Paul
^ permalink raw reply
* Re: [PATCH net-next] net: Separate the close_list and the unreg_list
From: Francesco Ruggeri @ 2013-10-10 0:22 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: netdev
In-Reply-To: <CA+HUmGg2LCX0zZAXgNhWwafBgi1HuePTLmxDu5VTUm7tX4TYMA@mail.gmail.com>
On Sun, Oct 6, 2013 at 10:13 AM, Francesco Ruggeri
<fruggeri@aristanetworks.com> wrote:
>>
>> Can you verify this incremental change fixes it for you?
>>
>
> Sure, I will.
> Thanks,
>
> Francesco
Hi Eric,
I have been running this patch for a few days and I have not had any problems.
Thanks,
Francesco
^ permalink raw reply
* Re: [PATCH] veth: Showing peer of veth type dev in ip link (kernel side)
From: Eric W. Biederman @ 2013-10-10 0:17 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, yamato, netdev
In-Reply-To: <20131009165254.2e1c8332@nehalam.linuxnetplumber.net>
Stephen Hemminger <stephen@networkplumber.org> writes:
> On Tue, 8 Oct 2013 14:13:37 -0700
> Stephen Hemminger <stephen@networkplumber.org> wrote:
>
>> On Tue, 08 Oct 2013 15:23:49 -0400 (EDT)
>> David Miller <davem@davemloft.net> wrote:
>>
>> > From: Masatake YAMATO <yamato@redhat.com>
>> > Date: Fri, 4 Oct 2013 11:34:21 +0900
>> >
>> > > ip link has ability to show extra information of net work device if
>> > > kernel provides sunh information. With this patch veth driver can
>> > > provide its peer ifindex information to ip command via netlink
>> > > interface.
>> > >
>> > > Signed-off-by: Masatake YAMATO <yamato@redhat.com>
>> >
>> > Applied to net-next, thank you.
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe netdev" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> Please revert this. It is incorrect.
>> The info returned by any netlink message should be equal to the message
>> for setting.
>>
>> I think the correct patch would be something like this (compile tested only).
>>
>> --- a/drivers/net/veth.c 2013-10-06 14:48:23.806461177 -0700
>> +++ b/drivers/net/veth.c 2013-10-08 14:11:42.434074690 -0700
>> @@ -434,6 +434,35 @@ static const struct nla_policy veth_poli
>> [VETH_INFO_PEER] = { .len = sizeof(struct ifinfomsg) },
>> };
>>
>> +static size_t veth_get_size(const struct net_device *dev)
>> +{
>> + return nla_total_size(sizeof(struct ifinfomsg)) + /* VETH_INFO_PEER */
>> + 0;
>> +}
>> +
>> +static int veth_fill_info(struct sk_buff *skb, const struct net_device *dev)
>> +{
>> + struct veth_priv *priv = netdev_priv(dev);
>> + struct net_device *peer = rtnl_dereference(priv->peer);
>> +
>> + if (peer) {
>> + struct ifinfomsg ifi = {
>> + .ifi_family = AF_UNSPEC,
>> + .ifi_type = peer->type,
>> + .ifi_index = peer->ifindex,
>> + .ifi_flags = dev_get_flags(peer),
>> + };
>> +
>> + if (nla_put(skb, VETH_INFO_PEER, sizeof(ifi), &ifi))
>> + goto nla_put_failure;
>> + }
>> +
>> + return 0;
>> +
>> +nla_put_failure:
>> + return -EMSGSIZE;
>> +}
>> +
>> static struct rtnl_link_ops veth_link_ops = {
>> .kind = DRV_NAME,
>> .priv_size = sizeof(struct veth_priv),
>> @@ -443,6 +472,8 @@ static struct rtnl_link_ops veth_link_op
>> .dellink = veth_dellink,
>> .policy = veth_policy,
>> .maxtype = VETH_INFO_MAX,
>> + .get_size = veth_get_size,
>> + .fill_info = veth_fill_info,
>> };
>>
>> /*
>>
>>
>
> This patch is ok as RFC starting point but the full implementation needs to
> add on IFLA_NAME and other attributes such that the full peer can be reconstructed.
>
> Ideally, the output of 'ip link' command can be in a format that can be used
> to recreate the same veth pair.
>
> One issue is that veth has the ability to make a peer in a different namespace
> and the network namespace code does not appear to have the ability to be invertable.
> I.e it is not possible to construct IFLA_NET_NS_PID or IFLA_NET_NS_FD attributes
> from an existing network device namespace.
Right.
IFLA_NET_NS_PID is not invertible as there may be no processes running
in a pid namespace.
IFLA_NET_NS_FD is in principle invertible. We just need to add a file
descriptor to the callers fd table. I don't see IFLA_NET_NS_FD being
invertible for broadcast messages, but for unicast it looks like a bit
of a pain but there are no fundamental problems.
I don't know if we care enough yet to write the code for the
IFLA_NET_NS_FD attribute but it is doable.
Eric
^ permalink raw reply
* [PATCH net-next] tcp: use ACCESS_ONCE() in tcp_update_pacing_rate()
From: Eric Dumazet @ 2013-10-10 0:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
sk_pacing_rate is read by sch_fq packet scheduler at any time,
with no synchronization, so make sure we update it in a
sensible way. ACCESS_ONCE() is how we instruct compiler
to not do stupid things, like using the memory location
as a temporary variable.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_input.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 47b8ab7..b441fe6 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -755,7 +755,12 @@ static void tcp_update_pacing_rate(struct sock *sk)
if (tp->srtt > 8 + 2)
do_div(rate, tp->srtt);
- sk->sk_pacing_rate = min_t(u64, rate, sk->sk_max_pacing_rate);
+ /* ACCESS_ONCE() is needed because sch_fq fetches sk_pacing_rate
+ * without any lock. We want to make sure compiler wont store
+ * intermediate values in this location.
+ */
+ ACCESS_ONCE(sk->sk_pacing_rate) = min_t(u64, rate,
+ sk->sk_max_pacing_rate);
}
/* Calculate rto without backoff. This is the second half of Van Jacobson's
^ permalink raw reply related
* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Eric Dumazet @ 2013-10-10 0:12 UTC (permalink / raw)
To: Josh Triplett
Cc: paulmck, linux-kernel, mingo, laijs, dipankar, akpm,
mathieu.desnoyers, niv, tglx, peterz, rostedt, dhowells, edumazet,
darren, fweisbec, sbw, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <20131009234040.GB14055@jtriplet-mobl1>
On Wed, 2013-10-09 at 16:40 -0700, Josh Triplett wrote:
> that. Constructs like list_del_rcu are much clearer, and not
> open-coded. Open-coding synchronization code is almost always a Bad
> Idea.
OK, so you think there is synchronization code.
I will shut up then, no need to waste time.
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 05/13] decnet: Apply rcu_access_pointer() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-09 23:57 UTC (permalink / raw)
To: Josh Triplett
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
niv, tglx, peterz, rostedt, dhowells, edumazet, darren, fweisbec,
sbw, David S. Miller, Thomas Graf, Gao feng, Stephen Hemminger,
linux-decnet-user, netdev
In-Reply-To: <20131009225716.GI11709@jtriplet-mobl1>
On Wed, Oct 09, 2013 at 03:57:16PM -0700, Josh Triplett wrote:
> On Wed, Oct 09, 2013 at 03:46:04PM -0700, Paul E. McKenney wrote:
> > On Wed, Oct 09, 2013 at 03:28:42PM -0700, Josh Triplett wrote:
> > > On Wed, Oct 09, 2013 at 02:29:38PM -0700, Paul E. McKenney wrote:
> > > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > >
> > > > The sparse checking for rcu_assign_pointer() was recently upgraded
> > > > to reject non-__kernel address spaces. This also rejects __rcu,
> > > > which is almost always the right thing to do. However, the use in
> > > > dn_insert_route() is legitimate: It is assigning a pointer to an element
> > >
> > > Nit: "uses ... are", not "use ... is". :)
> >
> > Don't I already have "use ... is"?
>
> I was suggesting that it needed to change from "use ... is" to "uses ...
> are", not the other way around.
I guess there really are two uses rather than just one. ;-)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 05/13] decnet: Apply rcu_access_pointer() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-09 23:54 UTC (permalink / raw)
To: Dhaval Giani
Cc: LKML, mingo, laijs, Dipankar, Andrew Morton, Mathieu Desnoyers,
josh, Nivedita Singhvi, Thomas Gleixner, Peter Zijlstra,
Steven Rostedt, David Howells, edumazet, Darren Hart,
Frederic Weisbecker, sbw, David S. Miller, Thomas Graf, Gao feng,
Stephen Hemminger, linux-decnet-user, netdev
In-Reply-To: <CAPhKKr8kLDNd7CdksHriyMzsqcj0oSf7mAfLceviMjobYGy7oA@mail.gmail.com>
On Wed, Oct 09, 2013 at 06:58:47PM -0400, Dhaval Giani wrote:
> On Wed, Oct 9, 2013 at 5:29 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> >
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >
> > The sparse checking for rcu_assign_pointer() was recently upgraded
> > to reject non-__kernel address spaces. This also rejects __rcu,
> > which is almost always the right thing to do. However, the use in
> > dn_insert_route() is legitimate: It is assigning a pointer to an element
> > from an RCU-protected list, and all elements of this list are already
> > visible to caller.
> >
> > This commit therefore silences this false positive by laundering the
> > pointer using rcu_access_pointer() as suggested by Josh Triplett.
> >
> > Reported-by: kbuild test robot <fengguang.wu@intel.com>
>
> I did not realize that we were allowed to rename people :-)
Copied and pasted directly from the email I received. Perhaps strange,
but true! ;-)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Josh Triplett @ 2013-10-09 23:40 UTC (permalink / raw)
To: Eric Dumazet
Cc: paulmck, linux-kernel, mingo, laijs, dipankar, akpm,
mathieu.desnoyers, niv, tglx, peterz, rostedt, dhowells, edumazet,
darren, fweisbec, sbw, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <1381360675.4971.45.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, Oct 09, 2013 at 04:17:55PM -0700, Eric Dumazet wrote:
> On Wed, 2013-10-09 at 15:56 -0700, Josh Triplett wrote:
>
> > I'd be really hesitant to introduce that type of direct assignment to an
> > __rcu pointer without wrapping it in some appropriately named macro, or
> > at the very least adding a comment.
>
> Well, there is no special magic here, in this specific case :
>
> - deleting an item in an rcu list
>
> Check list_del_rcu(), and you'll notice there is no _barrier_
>
> Adding correct barriers is good, but please do not add them when not
> needed.
>
> It makes code hard to understand.
I'm not arguing for the inclusion of an unnecessary barrier. I'm
arguing for something more self-documenting than:
> ACCESS(*ptr) = value;
that. Constructs like list_del_rcu are much clearer, and not
open-coded. Open-coding synchronization code is almost always a Bad
Idea.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Eric Dumazet @ 2013-10-09 23:17 UTC (permalink / raw)
To: Josh Triplett
Cc: paulmck, linux-kernel, mingo, laijs, dipankar, akpm,
mathieu.desnoyers, niv, tglx, peterz, rostedt, dhowells, edumazet,
darren, fweisbec, sbw, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <20131009225617.GH11709@jtriplet-mobl1>
On Wed, 2013-10-09 at 15:56 -0700, Josh Triplett wrote:
> I'd be really hesitant to introduce that type of direct assignment to an
> __rcu pointer without wrapping it in some appropriately named macro, or
> at the very least adding a comment.
Well, there is no special magic here, in this specific case :
- deleting an item in an rcu list
Check list_del_rcu(), and you'll notice there is no _barrier_
Adding correct barriers is good, but please do not add them when not
needed.
It makes code hard to understand.
ACCESS(*ptr) = value;
is clear and autodocumented, because it highlights the potential
problem, that is *ptr can be read without any barrier from another cpu.
So we ask the compiler to not write temporary garbage in it.
rcu_assign_pointer(*ptr, rcu_access_pointer(value))
is very confusing, because it hides the _real_ problem and add defensive
programming tricks.
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 05/13] decnet: Apply rcu_access_pointer() to avoid sparse false positive
From: Dhaval Giani @ 2013-10-09 22:58 UTC (permalink / raw)
To: Paul E. McKenney
Cc: LKML, mingo, laijs, Dipankar, Andrew Morton, Mathieu Desnoyers,
josh, Nivedita Singhvi, Thomas Gleixner, Peter Zijlstra,
Steven Rostedt, David Howells, edumazet, Darren Hart,
Frederic Weisbecker, sbw, David S. Miller, Thomas Graf, Gao feng,
Stephen Hemminger, linux-decnet-user, netdev
In-Reply-To: <1381354186-16285-5-git-send-email-paulmck@linux.vnet.ibm.com>
On Wed, Oct 9, 2013 at 5:29 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
>
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>
> The sparse checking for rcu_assign_pointer() was recently upgraded
> to reject non-__kernel address spaces. This also rejects __rcu,
> which is almost always the right thing to do. However, the use in
> dn_insert_route() is legitimate: It is assigning a pointer to an element
> from an RCU-protected list, and all elements of this list are already
> visible to caller.
>
> This commit therefore silences this false positive by laundering the
> pointer using rcu_access_pointer() as suggested by Josh Triplett.
>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
I did not realize that we were allowed to rename people :-)
Thanks!
Dhaval
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 05/13] decnet: Apply rcu_access_pointer() to avoid sparse false positive
From: Josh Triplett @ 2013-10-09 22:57 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
niv, tglx, peterz, rostedt, dhowells, edumazet, darren, fweisbec,
sbw, David S. Miller, Thomas Graf, Gao feng, Stephen Hemminger,
linux-decnet-user, netdev
In-Reply-To: <20131009224604.GF5790@linux.vnet.ibm.com>
On Wed, Oct 09, 2013 at 03:46:04PM -0700, Paul E. McKenney wrote:
> On Wed, Oct 09, 2013 at 03:28:42PM -0700, Josh Triplett wrote:
> > On Wed, Oct 09, 2013 at 02:29:38PM -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > >
> > > The sparse checking for rcu_assign_pointer() was recently upgraded
> > > to reject non-__kernel address spaces. This also rejects __rcu,
> > > which is almost always the right thing to do. However, the use in
> > > dn_insert_route() is legitimate: It is assigning a pointer to an element
> >
> > Nit: "uses ... are", not "use ... is". :)
>
> Don't I already have "use ... is"?
I was suggesting that it needed to change from "use ... is" to "uses ...
are", not the other way around.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Josh Triplett @ 2013-10-09 22:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: paulmck, linux-kernel, mingo, laijs, dipankar, akpm,
mathieu.desnoyers, niv, tglx, peterz, rostedt, dhowells, edumazet,
darren, fweisbec, sbw, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <1381359077.4971.37.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, Oct 09, 2013 at 03:51:17PM -0700, Eric Dumazet wrote:
> On Wed, 2013-10-09 at 15:36 -0700, Paul E. McKenney wrote:
>
> > That would work, though it would probably give sparse complaints.
>
> No sparse error here, as I said types are correct and SPARSE_RCU ready :
>
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index 583b77e..28f8495 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -245,7 +245,7 @@ ip6_tnl_unlink(struct ip6_tnl_net *ip6n, struct ip6_tnl *t)
> (iter = rtnl_dereference(*tp)) != NULL;
> tp = &iter->next) {
> if (t == iter) {
> - rcu_assign_pointer(*tp, t->next);
> + ACCESS_ONCE(*tp) = t->next;
> break;
> }
> }
I'd be really hesitant to introduce that type of direct assignment to an
__rcu pointer without wrapping it in some appropriately named macro, or
at the very least adding a comment.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Eric Dumazet @ 2013-10-09 22:51 UTC (permalink / raw)
To: paulmck
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, dhowells, edumazet, darren,
fweisbec, sbw, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <20131009223652.GC5790@linux.vnet.ibm.com>
On Wed, 2013-10-09 at 15:36 -0700, Paul E. McKenney wrote:
> That would work, though it would probably give sparse complaints.
No sparse error here, as I said types are correct and SPARSE_RCU ready :
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 583b77e..28f8495 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -245,7 +245,7 @@ ip6_tnl_unlink(struct ip6_tnl_net *ip6n, struct ip6_tnl *t)
(iter = rtnl_dereference(*tp)) != NULL;
tp = &iter->next) {
if (t == iter) {
- rcu_assign_pointer(*tp, t->next);
+ ACCESS_ONCE(*tp) = t->next;
break;
}
}
root@edumazet-glaptop:/usr/src/net-next# grep CONFIG_SPARSE_RCU_POINTER .config
CONFIG_SPARSE_RCU_POINTER=y
root@edumazet-glaptop:/usr/src/net-next# make C=2 net/ipv6/ip6_tunnel.o
make[1]: Nothing to be done for `all'.
make[1]: Nothing to be done for `relocs'.
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CALL scripts/checksyscalls.sh
CHECK scripts/mod/empty.c
CHECK net/ipv6/ip6_tunnel.c
^ permalink raw reply related
* Re: [PATCH v2 tip/core/rcu 0/13] Sparse-related updates for 3.13
From: Paul E. McKenney @ 2013-10-09 22:46 UTC (permalink / raw)
To: Josh Triplett
Cc: peterz, fweisbec, dhowells, edumazet, gaofeng, mingo, bridge,
jmorris, dipankar, darren, rostedt, niv, mathieu.desnoyers,
kuznet, tglx, johannes, laijs, yoshfuji, netdev,
linux-decnet-user, linux-kernel, kaber, stephen, sbw, tgraf, akpm,
fengguang.wu, davem
In-Reply-To: <20131009221805.GA11709@jtriplet-mobl1>
On Wed, Oct 09, 2013 at 03:18:05PM -0700, Josh Triplett wrote:
> On Wed, Oct 09, 2013 at 02:29:20PM -0700, Paul E. McKenney wrote:
> > Hello!
> >
> > This series features updates to allow sparse to do a better job of
> > statically analyzing RCU usage:
> >
> > 1. Apply ACCESS_ONCE() to rcu_assign_pointer()'s target to prevent
> > comiler mischief. Also require that the source pointer be from
> > the kernel address space. Sometimes it can be from the RCU address
> > space, which necessitates the remaining patches in this series.
> > Which, it must be admitted, apply to a very small fraction of
> > the rcu_assign_pointer() invocations in the kernel. This commit
> > courtesy of Josh Triplett.
> >
> > 2-13. Apply rcu_access_pointer() to avoid a number of false positives.
>
> I would suggest moving patch 1 to the end of the series, to avoid
> introducing and subsequently fixing warnings.
That would help with bisectability, will do!
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 05/13] decnet: Apply rcu_access_pointer() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-09 22:46 UTC (permalink / raw)
To: Josh Triplett
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
niv, tglx, peterz, rostedt, dhowells, edumazet, darren, fweisbec,
sbw, David S. Miller, Thomas Graf, Gao feng, Stephen Hemminger,
linux-decnet-user, netdev
In-Reply-To: <20131009222842.GD11709@jtriplet-mobl1>
On Wed, Oct 09, 2013 at 03:28:42PM -0700, Josh Triplett wrote:
> On Wed, Oct 09, 2013 at 02:29:38PM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >
> > The sparse checking for rcu_assign_pointer() was recently upgraded
> > to reject non-__kernel address spaces. This also rejects __rcu,
> > which is almost always the right thing to do. However, the use in
> > dn_insert_route() is legitimate: It is assigning a pointer to an element
>
> Nit: "uses ... are", not "use ... is". :)
Don't I already have "use ... is"?
Thanx, Paul
> > from an RCU-protected list, and all elements of this list are already
> > visible to caller.
> >
> > This commit therefore silences this false positive by laundering the
> > pointer using rcu_access_pointer() as suggested by Josh Triplett.
> >
> > Reported-by: kbuild test robot <fengguang.wu@intel.com>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Thomas Graf <tgraf@suug.ch>
> > Cc: Gao feng <gaofeng@cn.fujitsu.com>
> > Cc: Stephen Hemminger <shemminger@vyatta.com>
> > Cc: linux-decnet-user@lists.sourceforge.net
> > Cc: netdev@vger.kernel.org
>
> With or without the above typo fix:
>
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
>
> > ---
> > net/decnet/dn_route.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
> > index fe32388ea24f..3b1357bcfc92 100644
> > --- a/net/decnet/dn_route.c
> > +++ b/net/decnet/dn_route.c
> > @@ -345,7 +345,7 @@ static int dn_insert_route(struct dn_route *rt, unsigned int hash, struct dn_rou
> > /* Put it first */
> > *rthp = rth->dst.dn_next;
> > rcu_assign_pointer(rth->dst.dn_next,
> > - dn_rt_hash_table[hash].chain);
> > + rcu_access_pointer(dn_rt_hash_table[hash].chain));
> > rcu_assign_pointer(dn_rt_hash_table[hash].chain, rth);
> >
> > dst_use(&rth->dst, now);
> > @@ -358,7 +358,8 @@ static int dn_insert_route(struct dn_route *rt, unsigned int hash, struct dn_rou
> > rthp = &rth->dst.dn_next;
> > }
> >
> > - rcu_assign_pointer(rt->dst.dn_next, dn_rt_hash_table[hash].chain);
> > + rcu_assign_pointer(rt->dst.dn_next,
> > + rcu_access_pointer(dn_rt_hash_table[hash].chain));
> > rcu_assign_pointer(dn_rt_hash_table[hash].chain, rt);
> >
> > dst_use(&rt->dst, now);
> > --
> > 1.8.1.5
> >
>
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Paul E. McKenney @ 2013-10-09 22:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, dhowells, edumazet, darren,
fweisbec, sbw, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <1381356624.4971.26.camel@edumazet-glaptop.roam.corp.google.com>
On Wed, Oct 09, 2013 at 03:10:24PM -0700, Eric Dumazet wrote:
> On Wed, 2013-10-09 at 14:57 -0700, Paul E. McKenney wrote:
>
> > Hmmm... I could use RCU_INIT_POINTER(). Something like the following?
> >
> > RCU_INIT_POINTER(ACCESS_ONCE(*tp), t->next);
> >
> > The ACCESS_ONCE() to prevent the compiler from doing anything stupid.
> > Presumably the value of t->next cannot change, so a normal load suffices.
> >
> > Or did you have something else in mind?
>
> Well, *tp and t->next are both of the same type, with __rcu attribute.
>
> struct ip6_tnl __rcu **tp;
>
> So I meant :
>
> ACCESS_ONCE(*tp) = t->next;
>
> If really we can have a really stupid compiler.
That would work, though it would probably give sparse complaints.
Of course, it is not the stupid compilers that worry me, but rather the
smart ones...
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 0/13] Sparse-related updates for 3.13
From: Josh Triplett @ 2013-10-09 22:30 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
niv, tglx, peterz, rostedt, dhowells, edumazet, darren, fweisbec,
sbw, fengguang.wu, stephen, davem, bridge, netdev, tgraf, gaofeng,
linux-decnet-user, kuznet, jmorris, yoshfuji, kaber, linville,
johannes
In-Reply-To: <20131009212920.GA15413@linux.vnet.ibm.com>
On Wed, Oct 09, 2013 at 02:29:20PM -0700, Paul E. McKenney wrote:
> Hello!
>
> This series features updates to allow sparse to do a better job of
> statically analyzing RCU usage:
>
> 1. Apply ACCESS_ONCE() to rcu_assign_pointer()'s target to prevent
> comiler mischief. Also require that the source pointer be from
> the kernel address space. Sometimes it can be from the RCU address
> space, which necessitates the remaining patches in this series.
> Which, it must be admitted, apply to a very small fraction of
> the rcu_assign_pointer() invocations in the kernel. This commit
> courtesy of Josh Triplett.
>
> 2-13. Apply rcu_access_pointer() to avoid a number of false positives.
I posted one minor nit in response to one of these patches, but in any
case, for 2-13:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
I'm obviously OK with patch 1 as well, but it should move to the end of
the series, and you need a new patch 1 that adds a comment constraining
rcu_assign_pointer to evaluate its argument exactly once.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 05/13] decnet: Apply rcu_access_pointer() to avoid sparse false positive
From: Josh Triplett @ 2013-10-09 22:28 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
niv, tglx, peterz, rostedt, dhowells, edumazet, darren, fweisbec,
sbw, David S. Miller, Thomas Graf, Gao feng, Stephen Hemminger,
linux-decnet-user, netdev
In-Reply-To: <1381354186-16285-5-git-send-email-paulmck@linux.vnet.ibm.com>
On Wed, Oct 09, 2013 at 02:29:38PM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>
> The sparse checking for rcu_assign_pointer() was recently upgraded
> to reject non-__kernel address spaces. This also rejects __rcu,
> which is almost always the right thing to do. However, the use in
> dn_insert_route() is legitimate: It is assigning a pointer to an element
Nit: "uses ... are", not "use ... is". :)
> from an RCU-protected list, and all elements of this list are already
> visible to caller.
>
> This commit therefore silences this false positive by laundering the
> pointer using rcu_access_pointer() as suggested by Josh Triplett.
>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Thomas Graf <tgraf@suug.ch>
> Cc: Gao feng <gaofeng@cn.fujitsu.com>
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Cc: linux-decnet-user@lists.sourceforge.net
> Cc: netdev@vger.kernel.org
With or without the above typo fix:
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> ---
> net/decnet/dn_route.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
> index fe32388ea24f..3b1357bcfc92 100644
> --- a/net/decnet/dn_route.c
> +++ b/net/decnet/dn_route.c
> @@ -345,7 +345,7 @@ static int dn_insert_route(struct dn_route *rt, unsigned int hash, struct dn_rou
> /* Put it first */
> *rthp = rth->dst.dn_next;
> rcu_assign_pointer(rth->dst.dn_next,
> - dn_rt_hash_table[hash].chain);
> + rcu_access_pointer(dn_rt_hash_table[hash].chain));
> rcu_assign_pointer(dn_rt_hash_table[hash].chain, rth);
>
> dst_use(&rth->dst, now);
> @@ -358,7 +358,8 @@ static int dn_insert_route(struct dn_route *rt, unsigned int hash, struct dn_rou
> rthp = &rth->dst.dn_next;
> }
>
> - rcu_assign_pointer(rt->dst.dn_next, dn_rt_hash_table[hash].chain);
> + rcu_assign_pointer(rt->dst.dn_next,
> + rcu_access_pointer(dn_rt_hash_table[hash].chain));
> rcu_assign_pointer(dn_rt_hash_table[hash].chain, rt);
>
> dst_use(&rt->dst, now);
> --
> 1.8.1.5
>
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 0/13] Sparse-related updates for 3.13
From: Josh Triplett @ 2013-10-09 22:23 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
niv, tglx, peterz, rostedt, dhowells, edumazet, darren, fweisbec,
sbw, fengguang.wu, stephen, davem, bridge, netdev, tgraf, gaofeng,
linux-decnet-user, kuznet, jmorris, yoshfuji, kaber, linville,
johannes
In-Reply-To: <20131009212920.GA15413@linux.vnet.ibm.com>
On Wed, Oct 09, 2013 at 02:29:20PM -0700, Paul E. McKenney wrote:
> Hello!
>
> This series features updates to allow sparse to do a better job of
> statically analyzing RCU usage:
>
> 1. Apply ACCESS_ONCE() to rcu_assign_pointer()'s target to prevent
> comiler mischief. Also require that the source pointer be from
> the kernel address space. Sometimes it can be from the RCU address
> space, which necessitates the remaining patches in this series.
> Which, it must be admitted, apply to a very small fraction of
> the rcu_assign_pointer() invocations in the kernel. This commit
> courtesy of Josh Triplett.
>
> 2-13. Apply rcu_access_pointer() to avoid a number of false positives.
The use of rcu_access_pointer directly in the argument of
rcu_assign_pointer does add a new constraint to rcu_assign_pointer,
namely that it *must not* evaluate its argument more than once.
Currently, it expands its argument three times, but one expansion is
only visible to sparse (__CHECKER__), and one lives inside a typeof
(where it'll never be evaluated), so this is safe. However, I'd add a
comment to that effect above rcu_assign_pointer, explicitly saying that
it must evaluate its argument exactly once; that way, if anyone ever
changes it, they'll know they have to introduce an appropriate local
temporary variable inside the macro.
- Josh Triplett
^ permalink raw reply
* [PATCH net-next] inet: includes a sock_common in request_sock
From: Eric Dumazet @ 2013-10-09 22:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
TCP listener refactoring, part 5 :
We want to be able to insert request sockets (SYN_RECV) into main
ehash table instead of the per listener hash table to allow RCU
lookups and remove listener lock contention.
This patch includes the needed struct sock_common in front
of struct request_sock
This means there is no more inet6_request_sock IPv6 specific
structure.
Following inet_request_sock fields were renamed as they became
macros to reference fields from struct sock_common.
Prefix ir_ was chosen to avoid name collisions.
loc_port -> ir_loc_port
loc_addr -> ir_loc_addr
rmt_addr -> ir_rmt_addr
rmt_port -> ir_rmt_port
iif -> ir_iif
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/ipv6.h | 26 -----------
include/net/inet_sock.h | 16 ++++---
include/net/request_sock.h | 1
include/net/tcp.h | 4 -
net/dccp/ipv4.c | 18 ++++----
net/dccp/ipv6.c | 63 ++++++++++++++---------------
net/dccp/ipv6.h | 1
net/dccp/minisocks.c | 4 -
net/dccp/output.c | 4 -
net/ipv4/inet_connection_sock.c | 23 +++++-----
net/ipv4/inet_diag.c | 22 +++++-----
net/ipv4/syncookies.c | 12 ++---
net/ipv4/tcp_ipv4.c | 38 ++++++++---------
net/ipv4/tcp_metrics.c | 8 ++-
net/ipv4/tcp_output.c | 4 -
net/ipv6/inet6_connection_sock.c | 26 +++++------
net/ipv6/syncookies.c | 24 +++++------
net/ipv6/tcp_ipv6.c | 61 ++++++++++++++--------------
net/netlabel/netlabel_kapi.c | 2
19 files changed, 169 insertions(+), 188 deletions(-)
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 35f6c1b..a80a63c 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -115,16 +115,8 @@ static inline int inet6_iif(const struct sk_buff *skb)
return IP6CB(skb)->iif;
}
-struct inet6_request_sock {
- struct in6_addr loc_addr;
- struct in6_addr rmt_addr;
- struct sk_buff *pktopts;
- int iif;
-};
-
struct tcp6_request_sock {
struct tcp_request_sock tcp6rsk_tcp;
- struct inet6_request_sock tcp6rsk_inet6;
};
struct ipv6_mc_socklist;
@@ -264,26 +256,12 @@ static inline struct ipv6_pinfo * inet6_sk(const struct sock *__sk)
return inet_sk(__sk)->pinet6;
}
-static inline struct inet6_request_sock *
- inet6_rsk(const struct request_sock *rsk)
-{
- return (struct inet6_request_sock *)(((u8 *)rsk) +
- inet_rsk(rsk)->inet6_rsk_offset);
-}
-
-static inline u32 inet6_rsk_offset(struct request_sock *rsk)
-{
- return rsk->rsk_ops->obj_size - sizeof(struct inet6_request_sock);
-}
-
static inline struct request_sock *inet6_reqsk_alloc(struct request_sock_ops *ops)
{
struct request_sock *req = reqsk_alloc(ops);
- if (req != NULL) {
- inet_rsk(req)->inet6_rsk_offset = inet6_rsk_offset(req);
- inet6_rsk(req)->pktopts = NULL;
- }
+ if (req)
+ inet_rsk(req)->pktopts = NULL;
return req;
}
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 6d9a7e6..f912044 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -70,13 +70,14 @@ struct ip_options_data {
struct inet_request_sock {
struct request_sock req;
-#if IS_ENABLED(CONFIG_IPV6)
- u16 inet6_rsk_offset;
-#endif
- __be16 loc_port;
- __be32 loc_addr;
- __be32 rmt_addr;
- __be16 rmt_port;
+#define ir_loc_addr req.__req_common.skc_rcv_saddr
+#define ir_rmt_addr req.__req_common.skc_daddr
+#define ir_loc_port req.__req_common.skc_num
+#define ir_rmt_port req.__req_common.skc_dport
+#define ir_v6_rmt_addr req.__req_common.skc_v6_daddr
+#define ir_v6_loc_addr req.__req_common.skc_v6_rcv_saddr
+#define ir_iif req.__req_common.skc_bound_dev_if
+
kmemcheck_bitfield_begin(flags);
u16 snd_wscale : 4,
rcv_wscale : 4,
@@ -88,6 +89,7 @@ struct inet_request_sock {
no_srccheck: 1;
kmemcheck_bitfield_end(flags);
struct ip_options_rcu *opt;
+ struct sk_buff *pktopts;
};
static inline struct inet_request_sock *inet_rsk(const struct request_sock *sk)
diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index 65c3e516..7f830ff 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -48,6 +48,7 @@ int inet_rtx_syn_ack(struct sock *parent, struct request_sock *req);
/* struct request_sock - mini sock to represent a connection request
*/
struct request_sock {
+ struct sock_common __req_common;
struct request_sock *dl_next;
u16 mss;
u8 num_retrans; /* number of retransmits */
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 39bbfa1..24a0616 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1109,8 +1109,8 @@ static inline void tcp_openreq_init(struct request_sock *req,
ireq->wscale_ok = rx_opt->wscale_ok;
ireq->acked = 0;
ireq->ecn_ok = 0;
- ireq->rmt_port = tcp_hdr(skb)->source;
- ireq->loc_port = tcp_hdr(skb)->dest;
+ ireq->ir_rmt_port = tcp_hdr(skb)->source;
+ ireq->ir_loc_port = tcp_hdr(skb)->dest;
}
void tcp_enter_memory_pressure(struct sock *sk);
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index ebc54fe..720c362 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -409,9 +409,9 @@ struct sock *dccp_v4_request_recv_sock(struct sock *sk, struct sk_buff *skb,
newinet = inet_sk(newsk);
ireq = inet_rsk(req);
- newinet->inet_daddr = ireq->rmt_addr;
- newinet->inet_rcv_saddr = ireq->loc_addr;
- newinet->inet_saddr = ireq->loc_addr;
+ newinet->inet_daddr = ireq->ir_rmt_addr;
+ newinet->inet_rcv_saddr = ireq->ir_loc_addr;
+ newinet->inet_saddr = ireq->ir_loc_addr;
newinet->inet_opt = ireq->opt;
ireq->opt = NULL;
newinet->mc_index = inet_iif(skb);
@@ -516,10 +516,10 @@ static int dccp_v4_send_response(struct sock *sk, struct request_sock *req)
const struct inet_request_sock *ireq = inet_rsk(req);
struct dccp_hdr *dh = dccp_hdr(skb);
- dh->dccph_checksum = dccp_v4_csum_finish(skb, ireq->loc_addr,
- ireq->rmt_addr);
- err = ip_build_and_send_pkt(skb, sk, ireq->loc_addr,
- ireq->rmt_addr,
+ dh->dccph_checksum = dccp_v4_csum_finish(skb, ireq->ir_loc_addr,
+ ireq->ir_rmt_addr);
+ err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
+ ireq->ir_rmt_addr,
ireq->opt);
err = net_xmit_eval(err);
}
@@ -641,8 +641,8 @@ int dccp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
goto drop_and_free;
ireq = inet_rsk(req);
- ireq->loc_addr = ip_hdr(skb)->daddr;
- ireq->rmt_addr = ip_hdr(skb)->saddr;
+ ireq->ir_loc_addr = ip_hdr(skb)->daddr;
+ ireq->ir_rmt_addr = ip_hdr(skb)->saddr;
/*
* Step 3: Process LISTEN state
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 7f075b8..5cc5b24 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -216,7 +216,7 @@ out:
static int dccp_v6_send_response(struct sock *sk, struct request_sock *req)
{
- struct inet6_request_sock *ireq6 = inet6_rsk(req);
+ struct inet_request_sock *ireq = inet_rsk(req);
struct ipv6_pinfo *np = inet6_sk(sk);
struct sk_buff *skb;
struct in6_addr *final_p, final;
@@ -226,12 +226,12 @@ static int dccp_v6_send_response(struct sock *sk, struct request_sock *req)
memset(&fl6, 0, sizeof(fl6));
fl6.flowi6_proto = IPPROTO_DCCP;
- fl6.daddr = ireq6->rmt_addr;
- fl6.saddr = ireq6->loc_addr;
+ fl6.daddr = ireq->ir_v6_rmt_addr;
+ fl6.saddr = ireq->ir_v6_loc_addr;
fl6.flowlabel = 0;
- fl6.flowi6_oif = ireq6->iif;
- fl6.fl6_dport = inet_rsk(req)->rmt_port;
- fl6.fl6_sport = inet_rsk(req)->loc_port;
+ fl6.flowi6_oif = ireq->ir_iif;
+ fl6.fl6_dport = ireq->ir_rmt_port;
+ fl6.fl6_sport = ireq->ir_loc_port;
security_req_classify_flow(req, flowi6_to_flowi(&fl6));
@@ -249,9 +249,9 @@ static int dccp_v6_send_response(struct sock *sk, struct request_sock *req)
struct dccp_hdr *dh = dccp_hdr(skb);
dh->dccph_checksum = dccp_v6_csum_finish(skb,
- &ireq6->loc_addr,
- &ireq6->rmt_addr);
- fl6.daddr = ireq6->rmt_addr;
+ &ireq->ir_v6_loc_addr,
+ &ireq->ir_v6_rmt_addr);
+ fl6.daddr = ireq->ir_v6_rmt_addr;
err = ip6_xmit(sk, skb, &fl6, np->opt, np->tclass);
err = net_xmit_eval(err);
}
@@ -264,8 +264,7 @@ done:
static void dccp_v6_reqsk_destructor(struct request_sock *req)
{
dccp_feat_list_purge(&dccp_rsk(req)->dreq_featneg);
- if (inet6_rsk(req)->pktopts != NULL)
- kfree_skb(inet6_rsk(req)->pktopts);
+ kfree_skb(inet_rsk(req)->pktopts);
}
static void dccp_v6_ctl_send_reset(struct sock *sk, struct sk_buff *rxskb)
@@ -359,7 +358,7 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
{
struct request_sock *req;
struct dccp_request_sock *dreq;
- struct inet6_request_sock *ireq6;
+ struct inet_request_sock *ireq;
struct ipv6_pinfo *np = inet6_sk(sk);
const __be32 service = dccp_hdr_request(skb)->dccph_req_service;
struct dccp_skb_cb *dcb = DCCP_SKB_CB(skb);
@@ -398,22 +397,22 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
if (security_inet_conn_request(sk, skb, req))
goto drop_and_free;
- ireq6 = inet6_rsk(req);
- ireq6->rmt_addr = ipv6_hdr(skb)->saddr;
- ireq6->loc_addr = ipv6_hdr(skb)->daddr;
+ ireq = inet_rsk(req);
+ ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
+ ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
if (ipv6_opt_accepted(sk, skb) ||
np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim) {
atomic_inc(&skb->users);
- ireq6->pktopts = skb;
+ ireq->pktopts = skb;
}
- ireq6->iif = sk->sk_bound_dev_if;
+ ireq->ir_iif = sk->sk_bound_dev_if;
/* So that link locals have meaning */
if (!sk->sk_bound_dev_if &&
- ipv6_addr_type(&ireq6->rmt_addr) & IPV6_ADDR_LINKLOCAL)
- ireq6->iif = inet6_iif(skb);
+ ipv6_addr_type(&ireq->ir_v6_rmt_addr) & IPV6_ADDR_LINKLOCAL)
+ ireq->ir_iif = inet6_iif(skb);
/*
* Step 3: Process LISTEN state
@@ -446,7 +445,7 @@ static struct sock *dccp_v6_request_recv_sock(struct sock *sk,
struct request_sock *req,
struct dst_entry *dst)
{
- struct inet6_request_sock *ireq6 = inet6_rsk(req);
+ struct inet_request_sock *ireq = inet_rsk(req);
struct ipv6_pinfo *newnp, *np = inet6_sk(sk);
struct inet_sock *newinet;
struct dccp6_sock *newdp6;
@@ -505,12 +504,12 @@ static struct sock *dccp_v6_request_recv_sock(struct sock *sk,
memset(&fl6, 0, sizeof(fl6));
fl6.flowi6_proto = IPPROTO_DCCP;
- fl6.daddr = ireq6->rmt_addr;
+ fl6.daddr = ireq->ir_v6_rmt_addr;
final_p = fl6_update_dst(&fl6, np->opt, &final);
- fl6.saddr = ireq6->loc_addr;
+ fl6.saddr = ireq->ir_v6_loc_addr;
fl6.flowi6_oif = sk->sk_bound_dev_if;
- fl6.fl6_dport = inet_rsk(req)->rmt_port;
- fl6.fl6_sport = inet_rsk(req)->loc_port;
+ fl6.fl6_dport = ireq->ir_rmt_port;
+ fl6.fl6_sport = ireq->ir_loc_port;
security_sk_classify_flow(sk, flowi6_to_flowi(&fl6));
dst = ip6_dst_lookup_flow(sk, &fl6, final_p, false);
@@ -538,10 +537,10 @@ static struct sock *dccp_v6_request_recv_sock(struct sock *sk,
memcpy(newnp, np, sizeof(struct ipv6_pinfo));
- newsk->sk_v6_daddr = ireq6->rmt_addr;
- newnp->saddr = ireq6->loc_addr;
- newsk->sk_v6_rcv_saddr = ireq6->loc_addr;
- newsk->sk_bound_dev_if = ireq6->iif;
+ newsk->sk_v6_daddr = ireq->ir_v6_rmt_addr;
+ newnp->saddr = ireq->ir_v6_loc_addr;
+ newsk->sk_v6_rcv_saddr = ireq->ir_v6_loc_addr;
+ newsk->sk_bound_dev_if = ireq->ir_iif;
/* Now IPv6 options...
@@ -554,10 +553,10 @@ static struct sock *dccp_v6_request_recv_sock(struct sock *sk,
/* Clone pktoptions received with SYN */
newnp->pktoptions = NULL;
- if (ireq6->pktopts != NULL) {
- newnp->pktoptions = skb_clone(ireq6->pktopts, GFP_ATOMIC);
- consume_skb(ireq6->pktopts);
- ireq6->pktopts = NULL;
+ if (ireq->pktopts != NULL) {
+ newnp->pktoptions = skb_clone(ireq->pktopts, GFP_ATOMIC);
+ consume_skb(ireq->pktopts);
+ ireq->pktopts = NULL;
if (newnp->pktoptions)
skb_set_owner_r(newnp->pktoptions, newsk);
}
diff --git a/net/dccp/ipv6.h b/net/dccp/ipv6.h
index 6604fc3..af259e1 100644
--- a/net/dccp/ipv6.h
+++ b/net/dccp/ipv6.h
@@ -25,7 +25,6 @@ struct dccp6_sock {
struct dccp6_request_sock {
struct dccp_request_sock dccp;
- struct inet6_request_sock inet6;
};
struct dccp6_timewait_sock {
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index 32e80d9..66afbce 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -266,8 +266,8 @@ int dccp_reqsk_init(struct request_sock *req,
{
struct dccp_request_sock *dreq = dccp_rsk(req);
- inet_rsk(req)->rmt_port = dccp_hdr(skb)->dccph_sport;
- inet_rsk(req)->loc_port = dccp_hdr(skb)->dccph_dport;
+ inet_rsk(req)->ir_rmt_port = dccp_hdr(skb)->dccph_sport;
+ inet_rsk(req)->ir_loc_port = dccp_hdr(skb)->dccph_dport;
inet_rsk(req)->acked = 0;
dreq->dreq_timestamp_echo = 0;
diff --git a/net/dccp/output.c b/net/dccp/output.c
index d17fc90..9bf195d 100644
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -424,8 +424,8 @@ struct sk_buff *dccp_make_response(struct sock *sk, struct dst_entry *dst,
/* Build and checksum header */
dh = dccp_zeroed_hdr(skb, dccp_header_size);
- dh->dccph_sport = inet_rsk(req)->loc_port;
- dh->dccph_dport = inet_rsk(req)->rmt_port;
+ dh->dccph_sport = inet_rsk(req)->ir_loc_port;
+ dh->dccph_dport = inet_rsk(req)->ir_rmt_port;
dh->dccph_doff = (dccp_header_size +
DCCP_SKB_CB(skb)->dccpd_opt_len) / 4;
dh->dccph_type = DCCP_PKT_RESPONSE;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 56e82a4..2ffd931 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -412,8 +412,8 @@ struct dst_entry *inet_csk_route_req(struct sock *sk,
RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
sk->sk_protocol,
flags,
- (opt && opt->opt.srr) ? opt->opt.faddr : ireq->rmt_addr,
- ireq->loc_addr, ireq->rmt_port, inet_sk(sk)->inet_sport);
+ (opt && opt->opt.srr) ? opt->opt.faddr : ireq->ir_rmt_addr,
+ ireq->ir_loc_addr, ireq->ir_rmt_port, inet_sk(sk)->inet_sport);
security_req_classify_flow(req, flowi4_to_flowi(fl4));
rt = ip_route_output_flow(net, fl4, sk);
if (IS_ERR(rt))
@@ -448,8 +448,8 @@ struct dst_entry *inet_csk_route_child_sock(struct sock *sk,
flowi4_init_output(fl4, sk->sk_bound_dev_if, sk->sk_mark,
RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
sk->sk_protocol, inet_sk_flowi_flags(sk),
- (opt && opt->opt.srr) ? opt->opt.faddr : ireq->rmt_addr,
- ireq->loc_addr, ireq->rmt_port, inet_sk(sk)->inet_sport);
+ (opt && opt->opt.srr) ? opt->opt.faddr : ireq->ir_rmt_addr,
+ ireq->ir_loc_addr, ireq->ir_rmt_port, inet_sk(sk)->inet_sport);
security_req_classify_flow(req, flowi4_to_flowi(fl4));
rt = ip_route_output_flow(net, fl4, sk);
if (IS_ERR(rt))
@@ -495,9 +495,9 @@ struct request_sock *inet_csk_search_req(const struct sock *sk,
prev = &req->dl_next) {
const struct inet_request_sock *ireq = inet_rsk(req);
- if (ireq->rmt_port == rport &&
- ireq->rmt_addr == raddr &&
- ireq->loc_addr == laddr &&
+ if (ireq->ir_rmt_port == rport &&
+ ireq->ir_rmt_addr == raddr &&
+ ireq->ir_loc_addr == laddr &&
AF_INET_FAMILY(req->rsk_ops->family)) {
WARN_ON(req->sk);
*prevp = prev;
@@ -514,7 +514,8 @@ void inet_csk_reqsk_queue_hash_add(struct sock *sk, struct request_sock *req,
{
struct inet_connection_sock *icsk = inet_csk(sk);
struct listen_sock *lopt = icsk->icsk_accept_queue.listen_opt;
- const u32 h = inet_synq_hash(inet_rsk(req)->rmt_addr, inet_rsk(req)->rmt_port,
+ const u32 h = inet_synq_hash(inet_rsk(req)->ir_rmt_addr,
+ inet_rsk(req)->ir_rmt_port,
lopt->hash_rnd, lopt->nr_table_entries);
reqsk_queue_hash_req(&icsk->icsk_accept_queue, h, req, timeout);
@@ -674,9 +675,9 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
newsk->sk_state = TCP_SYN_RECV;
newicsk->icsk_bind_hash = NULL;
- inet_sk(newsk)->inet_dport = inet_rsk(req)->rmt_port;
- inet_sk(newsk)->inet_num = ntohs(inet_rsk(req)->loc_port);
- inet_sk(newsk)->inet_sport = inet_rsk(req)->loc_port;
+ inet_sk(newsk)->inet_dport = inet_rsk(req)->ir_rmt_port;
+ inet_sk(newsk)->inet_num = ntohs(inet_rsk(req)->ir_loc_port);
+ inet_sk(newsk)->inet_sport = inet_rsk(req)->ir_loc_port;
newsk->sk_write_space = sk_stream_write_space;
newicsk->icsk_retransmits = 0;
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index ecc179d..41e1c3e 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -679,12 +679,12 @@ static inline void inet_diag_req_addrs(const struct sock *sk,
#if IS_ENABLED(CONFIG_IPV6)
if (sk->sk_family == AF_INET6) {
if (req->rsk_ops->family == AF_INET6) {
- entry->saddr = inet6_rsk(req)->loc_addr.s6_addr32;
- entry->daddr = inet6_rsk(req)->rmt_addr.s6_addr32;
+ entry->saddr = ireq->ir_v6_loc_addr.s6_addr32;
+ entry->daddr = ireq->ir_v6_rmt_addr.s6_addr32;
} else if (req->rsk_ops->family == AF_INET) {
- ipv6_addr_set_v4mapped(ireq->loc_addr,
+ ipv6_addr_set_v4mapped(ireq->ir_loc_addr,
&entry->saddr_storage);
- ipv6_addr_set_v4mapped(ireq->rmt_addr,
+ ipv6_addr_set_v4mapped(ireq->ir_rmt_addr,
&entry->daddr_storage);
entry->saddr = entry->saddr_storage.s6_addr32;
entry->daddr = entry->daddr_storage.s6_addr32;
@@ -692,8 +692,8 @@ static inline void inet_diag_req_addrs(const struct sock *sk,
} else
#endif
{
- entry->saddr = &ireq->loc_addr;
- entry->daddr = &ireq->rmt_addr;
+ entry->saddr = &ireq->ir_loc_addr;
+ entry->daddr = &ireq->ir_rmt_addr;
}
}
@@ -728,9 +728,9 @@ static int inet_diag_fill_req(struct sk_buff *skb, struct sock *sk,
tmo = 0;
r->id.idiag_sport = inet->inet_sport;
- r->id.idiag_dport = ireq->rmt_port;
- r->id.idiag_src[0] = ireq->loc_addr;
- r->id.idiag_dst[0] = ireq->rmt_addr;
+ r->id.idiag_dport = ireq->ir_rmt_port;
+ r->id.idiag_src[0] = ireq->ir_loc_addr;
+ r->id.idiag_dst[0] = ireq->ir_rmt_addr;
r->idiag_expires = jiffies_to_msecs(tmo);
r->idiag_rqueue = 0;
r->idiag_wqueue = 0;
@@ -789,13 +789,13 @@ static int inet_diag_dump_reqs(struct sk_buff *skb, struct sock *sk,
if (reqnum < s_reqnum)
continue;
- if (r->id.idiag_dport != ireq->rmt_port &&
+ if (r->id.idiag_dport != ireq->ir_rmt_port &&
r->id.idiag_dport)
continue;
if (bc) {
inet_diag_req_addrs(sk, req, &entry);
- entry.dport = ntohs(ireq->rmt_port);
+ entry.dport = ntohs(ireq->ir_rmt_port);
if (!inet_diag_bc_run(bc, &entry))
continue;
diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 15e0241..984e21c 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -304,10 +304,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
treq->rcv_isn = ntohl(th->seq) - 1;
treq->snt_isn = cookie;
req->mss = mss;
- ireq->loc_port = th->dest;
- ireq->rmt_port = th->source;
- ireq->loc_addr = ip_hdr(skb)->daddr;
- ireq->rmt_addr = ip_hdr(skb)->saddr;
+ ireq->ir_loc_port = th->dest;
+ ireq->ir_rmt_port = th->source;
+ ireq->ir_loc_addr = ip_hdr(skb)->daddr;
+ ireq->ir_rmt_addr = ip_hdr(skb)->saddr;
ireq->ecn_ok = ecn_ok;
ireq->snd_wscale = tcp_opt.snd_wscale;
ireq->sack_ok = tcp_opt.sack_ok;
@@ -347,8 +347,8 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
flowi4_init_output(&fl4, sk->sk_bound_dev_if, sk->sk_mark,
RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE, IPPROTO_TCP,
inet_sk_flowi_flags(sk),
- (opt && opt->srr) ? opt->faddr : ireq->rmt_addr,
- ireq->loc_addr, th->source, th->dest);
+ (opt && opt->srr) ? opt->faddr : ireq->ir_rmt_addr,
+ ireq->ir_loc_addr, th->source, th->dest);
security_req_classify_flow(req, flowi4_to_flowi(&fl4));
rt = ip_route_output_key(sock_net(sk), &fl4);
if (IS_ERR(rt)) {
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index e4695dd..114d1b74 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -835,11 +835,11 @@ static int tcp_v4_send_synack(struct sock *sk, struct dst_entry *dst,
skb = tcp_make_synack(sk, dst, req, NULL);
if (skb) {
- __tcp_v4_send_check(skb, ireq->loc_addr, ireq->rmt_addr);
+ __tcp_v4_send_check(skb, ireq->ir_loc_addr, ireq->ir_rmt_addr);
skb_set_queue_mapping(skb, queue_mapping);
- err = ip_build_and_send_pkt(skb, sk, ireq->loc_addr,
- ireq->rmt_addr,
+ err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
+ ireq->ir_rmt_addr,
ireq->opt);
err = net_xmit_eval(err);
if (!tcp_rsk(req)->snt_synack && !err)
@@ -972,7 +972,7 @@ static struct tcp_md5sig_key *tcp_v4_reqsk_md5_lookup(struct sock *sk,
{
union tcp_md5_addr *addr;
- addr = (union tcp_md5_addr *)&inet_rsk(req)->rmt_addr;
+ addr = (union tcp_md5_addr *)&inet_rsk(req)->ir_rmt_addr;
return tcp_md5_do_lookup(sk, addr, AF_INET);
}
@@ -1149,8 +1149,8 @@ int tcp_v4_md5_hash_skb(char *md5_hash, struct tcp_md5sig_key *key,
saddr = inet_sk(sk)->inet_saddr;
daddr = inet_sk(sk)->inet_daddr;
} else if (req) {
- saddr = inet_rsk(req)->loc_addr;
- daddr = inet_rsk(req)->rmt_addr;
+ saddr = inet_rsk(req)->ir_loc_addr;
+ daddr = inet_rsk(req)->ir_rmt_addr;
} else {
const struct iphdr *iph = ip_hdr(skb);
saddr = iph->saddr;
@@ -1366,8 +1366,8 @@ static int tcp_v4_conn_req_fastopen(struct sock *sk,
kfree_skb(skb_synack);
return -1;
}
- err = ip_build_and_send_pkt(skb_synack, sk, ireq->loc_addr,
- ireq->rmt_addr, ireq->opt);
+ err = ip_build_and_send_pkt(skb_synack, sk, ireq->ir_loc_addr,
+ ireq->ir_rmt_addr, ireq->opt);
err = net_xmit_eval(err);
if (!err)
tcp_rsk(req)->snt_synack = tcp_time_stamp;
@@ -1502,8 +1502,8 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
tcp_openreq_init(req, &tmp_opt, skb);
ireq = inet_rsk(req);
- ireq->loc_addr = daddr;
- ireq->rmt_addr = saddr;
+ ireq->ir_loc_addr = daddr;
+ ireq->ir_rmt_addr = saddr;
ireq->no_srccheck = inet_sk(sk)->transparent;
ireq->opt = tcp_v4_save_options(skb);
@@ -1578,15 +1578,15 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
fastopen_cookie_present(&valid_foc) ? &valid_foc : NULL);
if (skb_synack) {
- __tcp_v4_send_check(skb_synack, ireq->loc_addr, ireq->rmt_addr);
+ __tcp_v4_send_check(skb_synack, ireq->ir_loc_addr, ireq->ir_rmt_addr);
skb_set_queue_mapping(skb_synack, skb_get_queue_mapping(skb));
} else
goto drop_and_free;
if (likely(!do_fastopen)) {
int err;
- err = ip_build_and_send_pkt(skb_synack, sk, ireq->loc_addr,
- ireq->rmt_addr, ireq->opt);
+ err = ip_build_and_send_pkt(skb_synack, sk, ireq->ir_loc_addr,
+ ireq->ir_rmt_addr, ireq->opt);
err = net_xmit_eval(err);
if (err || want_cookie)
goto drop_and_free;
@@ -1644,9 +1644,9 @@ struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
newtp = tcp_sk(newsk);
newinet = inet_sk(newsk);
ireq = inet_rsk(req);
- newinet->inet_daddr = ireq->rmt_addr;
- newinet->inet_rcv_saddr = ireq->loc_addr;
- newinet->inet_saddr = ireq->loc_addr;
+ newinet->inet_daddr = ireq->ir_rmt_addr;
+ newinet->inet_rcv_saddr = ireq->ir_loc_addr;
+ newinet->inet_saddr = ireq->ir_loc_addr;
inet_opt = ireq->opt;
rcu_assign_pointer(newinet->inet_opt, inet_opt);
ireq->opt = NULL;
@@ -2548,10 +2548,10 @@ static void get_openreq4(const struct sock *sk, const struct request_sock *req,
seq_printf(f, "%4d: %08X:%04X %08X:%04X"
" %02X %08X:%08X %02X:%08lX %08X %5u %8d %u %d %pK%n",
i,
- ireq->loc_addr,
+ ireq->ir_loc_addr,
ntohs(inet_sk(sk)->inet_sport),
- ireq->rmt_addr,
- ntohs(ireq->rmt_port),
+ ireq->ir_rmt_addr,
+ ntohs(ireq->ir_rmt_port),
TCP_SYN_RECV,
0, 0, /* could print option size, but that is af dependent. */
1, /* timers active (only the expire timer) */
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 8fcc2cb..4a2a841 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -215,13 +215,15 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
addr.family = req->rsk_ops->family;
switch (addr.family) {
case AF_INET:
- addr.addr.a4 = inet_rsk(req)->rmt_addr;
+ addr.addr.a4 = inet_rsk(req)->ir_rmt_addr;
hash = (__force unsigned int) addr.addr.a4;
break;
+#if IS_ENABLED(CONFIG_IPV6)
case AF_INET6:
- *(struct in6_addr *)addr.addr.a6 = inet6_rsk(req)->rmt_addr;
- hash = ipv6_addr_hash(&inet6_rsk(req)->rmt_addr);
+ *(struct in6_addr *)addr.addr.a6 = inet_rsk(req)->ir_v6_rmt_addr;
+ hash = ipv6_addr_hash(&inet_rsk(req)->ir_v6_rmt_addr);
break;
+#endif
default:
return NULL;
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index c6f01f2..faec813 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2734,8 +2734,8 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
th->syn = 1;
th->ack = 1;
TCP_ECN_make_synack(req, th);
- th->source = ireq->loc_port;
- th->dest = ireq->rmt_port;
+ th->source = ireq->ir_loc_port;
+ th->dest = ireq->ir_rmt_port;
/* Setting of flags are superfluous here for callers (and ECE is
* not even correctly set)
*/
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index b7400b4..1317c56 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -70,20 +70,20 @@ struct dst_entry *inet6_csk_route_req(struct sock *sk,
struct flowi6 *fl6,
const struct request_sock *req)
{
- struct inet6_request_sock *treq = inet6_rsk(req);
+ struct inet_request_sock *ireq = inet_rsk(req);
struct ipv6_pinfo *np = inet6_sk(sk);
struct in6_addr *final_p, final;
struct dst_entry *dst;
memset(fl6, 0, sizeof(*fl6));
fl6->flowi6_proto = IPPROTO_TCP;
- fl6->daddr = treq->rmt_addr;
+ fl6->daddr = ireq->ir_v6_rmt_addr;
final_p = fl6_update_dst(fl6, np->opt, &final);
- fl6->saddr = treq->loc_addr;
- fl6->flowi6_oif = treq->iif;
+ fl6->saddr = ireq->ir_v6_loc_addr;
+ fl6->flowi6_oif = ireq->ir_iif;
fl6->flowi6_mark = sk->sk_mark;
- fl6->fl6_dport = inet_rsk(req)->rmt_port;
- fl6->fl6_sport = inet_rsk(req)->loc_port;
+ fl6->fl6_dport = ireq->ir_rmt_port;
+ fl6->fl6_sport = ireq->ir_loc_port;
security_req_classify_flow(req, flowi6_to_flowi(fl6));
dst = ip6_dst_lookup_flow(sk, fl6, final_p, false);
@@ -129,13 +129,13 @@ struct request_sock *inet6_csk_search_req(const struct sock *sk,
lopt->nr_table_entries)];
(req = *prev) != NULL;
prev = &req->dl_next) {
- const struct inet6_request_sock *treq = inet6_rsk(req);
+ const struct inet_request_sock *ireq = inet_rsk(req);
- if (inet_rsk(req)->rmt_port == rport &&
+ if (ireq->ir_rmt_port == rport &&
req->rsk_ops->family == AF_INET6 &&
- ipv6_addr_equal(&treq->rmt_addr, raddr) &&
- ipv6_addr_equal(&treq->loc_addr, laddr) &&
- (!treq->iif || treq->iif == iif)) {
+ ipv6_addr_equal(&ireq->ir_v6_rmt_addr, raddr) &&
+ ipv6_addr_equal(&ireq->ir_v6_loc_addr, laddr) &&
+ (!ireq->ir_iif || ireq->ir_iif == iif)) {
WARN_ON(req->sk != NULL);
*prevp = prev;
return req;
@@ -153,8 +153,8 @@ void inet6_csk_reqsk_queue_hash_add(struct sock *sk,
{
struct inet_connection_sock *icsk = inet_csk(sk);
struct listen_sock *lopt = icsk->icsk_accept_queue.listen_opt;
- const u32 h = inet6_synq_hash(&inet6_rsk(req)->rmt_addr,
- inet_rsk(req)->rmt_port,
+ const u32 h = inet6_synq_hash(&inet_rsk(req)->ir_v6_rmt_addr,
+ inet_rsk(req)->ir_rmt_port,
lopt->hash_rnd, lopt->nr_table_entries);
reqsk_queue_hash_req(&icsk->icsk_accept_queue, h, req, timeout);
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index d703218..bc5698f9 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -150,7 +150,6 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
{
struct tcp_options_received tcp_opt;
struct inet_request_sock *ireq;
- struct inet6_request_sock *ireq6;
struct tcp_request_sock *treq;
struct ipv6_pinfo *np = inet6_sk(sk);
struct tcp_sock *tp = tcp_sk(sk);
@@ -187,7 +186,6 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
goto out;
ireq = inet_rsk(req);
- ireq6 = inet6_rsk(req);
treq = tcp_rsk(req);
treq->listener = NULL;
@@ -195,22 +193,22 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
goto out_free;
req->mss = mss;
- ireq->rmt_port = th->source;
- ireq->loc_port = th->dest;
- ireq6->rmt_addr = ipv6_hdr(skb)->saddr;
- ireq6->loc_addr = ipv6_hdr(skb)->daddr;
+ ireq->ir_rmt_port = th->source;
+ ireq->ir_loc_port = th->dest;
+ ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
+ ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
if (ipv6_opt_accepted(sk, skb) ||
np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim) {
atomic_inc(&skb->users);
- ireq6->pktopts = skb;
+ ireq->pktopts = skb;
}
- ireq6->iif = sk->sk_bound_dev_if;
+ ireq->ir_iif = sk->sk_bound_dev_if;
/* So that link locals have meaning */
if (!sk->sk_bound_dev_if &&
- ipv6_addr_type(&ireq6->rmt_addr) & IPV6_ADDR_LINKLOCAL)
- ireq6->iif = inet6_iif(skb);
+ ipv6_addr_type(&ireq->ir_v6_rmt_addr) & IPV6_ADDR_LINKLOCAL)
+ ireq->ir_iif = inet6_iif(skb);
req->expires = 0UL;
req->num_retrans = 0;
@@ -234,12 +232,12 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
struct flowi6 fl6;
memset(&fl6, 0, sizeof(fl6));
fl6.flowi6_proto = IPPROTO_TCP;
- fl6.daddr = ireq6->rmt_addr;
+ fl6.daddr = ireq->ir_v6_rmt_addr;
final_p = fl6_update_dst(&fl6, np->opt, &final);
- fl6.saddr = ireq6->loc_addr;
+ fl6.saddr = ireq->ir_v6_loc_addr;
fl6.flowi6_oif = sk->sk_bound_dev_if;
fl6.flowi6_mark = sk->sk_mark;
- fl6.fl6_dport = inet_rsk(req)->rmt_port;
+ fl6.fl6_dport = ireq->ir_rmt_port;
fl6.fl6_sport = inet_sk(sk)->inet_sport;
security_req_classify_flow(req, flowi6_to_flowi(&fl6));
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 541dfc4..db234d6 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -465,7 +465,7 @@ static int tcp_v6_send_synack(struct sock *sk, struct dst_entry *dst,
struct request_sock *req,
u16 queue_mapping)
{
- struct inet6_request_sock *treq = inet6_rsk(req);
+ struct inet_request_sock *ireq = inet_rsk(req);
struct ipv6_pinfo *np = inet6_sk(sk);
struct sk_buff * skb;
int err = -ENOMEM;
@@ -477,9 +477,10 @@ static int tcp_v6_send_synack(struct sock *sk, struct dst_entry *dst,
skb = tcp_make_synack(sk, dst, req, NULL);
if (skb) {
- __tcp_v6_send_check(skb, &treq->loc_addr, &treq->rmt_addr);
+ __tcp_v6_send_check(skb, &ireq->ir_v6_loc_addr,
+ &ireq->ir_v6_rmt_addr);
- fl6->daddr = treq->rmt_addr;
+ fl6->daddr = ireq->ir_v6_rmt_addr;
skb_set_queue_mapping(skb, queue_mapping);
err = ip6_xmit(sk, skb, fl6, np->opt, np->tclass);
err = net_xmit_eval(err);
@@ -502,7 +503,7 @@ static int tcp_v6_rtx_synack(struct sock *sk, struct request_sock *req)
static void tcp_v6_reqsk_destructor(struct request_sock *req)
{
- kfree_skb(inet6_rsk(req)->pktopts);
+ kfree_skb(inet_rsk(req)->pktopts);
}
#ifdef CONFIG_TCP_MD5SIG
@@ -521,7 +522,7 @@ static struct tcp_md5sig_key *tcp_v6_md5_lookup(struct sock *sk,
static struct tcp_md5sig_key *tcp_v6_reqsk_md5_lookup(struct sock *sk,
struct request_sock *req)
{
- return tcp_v6_md5_do_lookup(sk, &inet6_rsk(req)->rmt_addr);
+ return tcp_v6_md5_do_lookup(sk, &inet_rsk(req)->ir_v6_rmt_addr);
}
static int tcp_v6_parse_md5_keys (struct sock *sk, char __user *optval,
@@ -623,8 +624,8 @@ static int tcp_v6_md5_hash_skb(char *md5_hash, struct tcp_md5sig_key *key,
saddr = &inet6_sk(sk)->saddr;
daddr = &sk->sk_v6_daddr;
} else if (req) {
- saddr = &inet6_rsk(req)->loc_addr;
- daddr = &inet6_rsk(req)->rmt_addr;
+ saddr = &inet_rsk(req)->ir_v6_loc_addr;
+ daddr = &inet_rsk(req)->ir_v6_rmt_addr;
} else {
const struct ipv6hdr *ip6h = ipv6_hdr(skb);
saddr = &ip6h->saddr;
@@ -949,7 +950,7 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
{
struct tcp_options_received tmp_opt;
struct request_sock *req;
- struct inet6_request_sock *treq;
+ struct inet_request_sock *ireq;
struct ipv6_pinfo *np = inet6_sk(sk);
struct tcp_sock *tp = tcp_sk(sk);
__u32 isn = TCP_SKB_CB(skb)->when;
@@ -994,25 +995,25 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
tmp_opt.tstamp_ok = tmp_opt.saw_tstamp;
tcp_openreq_init(req, &tmp_opt, skb);
- treq = inet6_rsk(req);
- treq->rmt_addr = ipv6_hdr(skb)->saddr;
- treq->loc_addr = ipv6_hdr(skb)->daddr;
+ ireq = inet_rsk(req);
+ ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
+ ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
if (!want_cookie || tmp_opt.tstamp_ok)
TCP_ECN_create_request(req, skb, sock_net(sk));
- treq->iif = sk->sk_bound_dev_if;
+ ireq->ir_iif = sk->sk_bound_dev_if;
/* So that link locals have meaning */
if (!sk->sk_bound_dev_if &&
- ipv6_addr_type(&treq->rmt_addr) & IPV6_ADDR_LINKLOCAL)
- treq->iif = inet6_iif(skb);
+ ipv6_addr_type(&ireq->ir_v6_rmt_addr) & IPV6_ADDR_LINKLOCAL)
+ ireq->ir_iif = inet6_iif(skb);
if (!isn) {
if (ipv6_opt_accepted(sk, skb) ||
np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim) {
atomic_inc(&skb->users);
- treq->pktopts = skb;
+ ireq->pktopts = skb;
}
if (want_cookie) {
@@ -1051,7 +1052,7 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
* to the moment of synflood.
*/
LIMIT_NETDEBUG(KERN_DEBUG "TCP: drop open request from %pI6/%u\n",
- &treq->rmt_addr, ntohs(tcp_hdr(skb)->source));
+ &ireq->ir_v6_rmt_addr, ntohs(tcp_hdr(skb)->source));
goto drop_and_release;
}
@@ -1086,7 +1087,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
struct dst_entry *dst)
{
- struct inet6_request_sock *treq;
+ struct inet_request_sock *ireq;
struct ipv6_pinfo *newnp, *np = inet6_sk(sk);
struct tcp6_sock *newtcp6sk;
struct inet_sock *newinet;
@@ -1151,7 +1152,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
return newsk;
}
- treq = inet6_rsk(req);
+ ireq = inet_rsk(req);
if (sk_acceptq_is_full(sk))
goto out_overflow;
@@ -1185,10 +1186,10 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
memcpy(newnp, np, sizeof(struct ipv6_pinfo));
- newsk->sk_v6_daddr = treq->rmt_addr;
- newnp->saddr = treq->loc_addr;
- newsk->sk_v6_rcv_saddr = treq->loc_addr;
- newsk->sk_bound_dev_if = treq->iif;
+ newsk->sk_v6_daddr = ireq->ir_v6_rmt_addr;
+ newnp->saddr = ireq->ir_v6_loc_addr;
+ newsk->sk_v6_rcv_saddr = ireq->ir_v6_loc_addr;
+ newsk->sk_bound_dev_if = ireq->ir_iif;
/* Now IPv6 options...
@@ -1203,11 +1204,11 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
/* Clone pktoptions received with SYN */
newnp->pktoptions = NULL;
- if (treq->pktopts != NULL) {
- newnp->pktoptions = skb_clone(treq->pktopts,
+ if (ireq->pktopts != NULL) {
+ newnp->pktoptions = skb_clone(ireq->pktopts,
sk_gfp_atomic(sk, GFP_ATOMIC));
- consume_skb(treq->pktopts);
- treq->pktopts = NULL;
+ consume_skb(ireq->pktopts);
+ ireq->pktopts = NULL;
if (newnp->pktoptions)
skb_set_owner_r(newnp->pktoptions, newsk);
}
@@ -1722,8 +1723,8 @@ static void get_openreq6(struct seq_file *seq,
const struct sock *sk, struct request_sock *req, int i, kuid_t uid)
{
int ttd = req->expires - jiffies;
- const struct in6_addr *src = &inet6_rsk(req)->loc_addr;
- const struct in6_addr *dest = &inet6_rsk(req)->rmt_addr;
+ const struct in6_addr *src = &inet_rsk(req)->ir_v6_loc_addr;
+ const struct in6_addr *dest = &inet_rsk(req)->ir_v6_rmt_addr;
if (ttd < 0)
ttd = 0;
@@ -1734,10 +1735,10 @@ static void get_openreq6(struct seq_file *seq,
i,
src->s6_addr32[0], src->s6_addr32[1],
src->s6_addr32[2], src->s6_addr32[3],
- ntohs(inet_rsk(req)->loc_port),
+ ntohs(inet_rsk(req)->ir_loc_port),
dest->s6_addr32[0], dest->s6_addr32[1],
dest->s6_addr32[2], dest->s6_addr32[3],
- ntohs(inet_rsk(req)->rmt_port),
+ ntohs(inet_rsk(req)->ir_rmt_port),
TCP_SYN_RECV,
0,0, /* could print option size, but that is af dependent. */
1, /* timers active (only the expire timer) */
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 96a458e..dce1beb 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -817,7 +817,7 @@ int netlbl_req_setattr(struct request_sock *req,
switch (req->rsk_ops->family) {
case AF_INET:
entry = netlbl_domhsh_getentry_af4(secattr->domain,
- inet_rsk(req)->rmt_addr);
+ inet_rsk(req)->ir_rmt_addr);
if (entry == NULL) {
ret_val = -ENOENT;
goto req_setattr_return;
^ permalink raw reply related
* Re: [PATCH v2 tip/core/rcu 0/13] Sparse-related updates for 3.13
From: Josh Triplett @ 2013-10-09 22:18 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
niv, tglx, peterz, rostedt, dhowells, edumazet, darren, fweisbec,
sbw, fengguang.wu, stephen, davem, bridge, netdev, tgraf, gaofeng,
linux-decnet-user, kuznet, jmorris, yoshfuji, kaber, linville,
johannes
In-Reply-To: <20131009212920.GA15413@linux.vnet.ibm.com>
On Wed, Oct 09, 2013 at 02:29:20PM -0700, Paul E. McKenney wrote:
> Hello!
>
> This series features updates to allow sparse to do a better job of
> statically analyzing RCU usage:
>
> 1. Apply ACCESS_ONCE() to rcu_assign_pointer()'s target to prevent
> comiler mischief. Also require that the source pointer be from
> the kernel address space. Sometimes it can be from the RCU address
> space, which necessitates the remaining patches in this series.
> Which, it must be admitted, apply to a very small fraction of
> the rcu_assign_pointer() invocations in the kernel. This commit
> courtesy of Josh Triplett.
>
> 2-13. Apply rcu_access_pointer() to avoid a number of false positives.
I would suggest moving patch 1 to the end of the series, to avoid
introducing and subsequently fixing warnings.
- Josh Triplett
^ permalink raw reply
* Re: [PATCH v2 tip/core/rcu 07/13] ipv6/ip6_tunnel: Apply rcu_access_pointer() to avoid sparse false positive
From: Eric Dumazet @ 2013-10-09 22:10 UTC (permalink / raw)
To: paulmck
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, dhowells, edumazet, darren,
fweisbec, sbw, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
In-Reply-To: <20131009215747.GA5790@linux.vnet.ibm.com>
On Wed, 2013-10-09 at 14:57 -0700, Paul E. McKenney wrote:
> Hmmm... I could use RCU_INIT_POINTER(). Something like the following?
>
> RCU_INIT_POINTER(ACCESS_ONCE(*tp), t->next);
>
> The ACCESS_ONCE() to prevent the compiler from doing anything stupid.
> Presumably the value of t->next cannot change, so a normal load suffices.
>
> Or did you have something else in mind?
Well, *tp and t->next are both of the same type, with __rcu attribute.
struct ip6_tnl __rcu **tp;
So I meant :
ACCESS_ONCE(*tp) = t->next;
If really we can have a really stupid compiler.
^ 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