lkml.org 
[lkml]   [2018]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] move smp_rmb() after load of status
Date
Signed-off-by: humbabek <humbabek@gmail.com>
---
net/packet/af_packet.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 57634bc3da74..91830ef07c48 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -394,25 +394,30 @@ static void __packet_set_status(struct packet_sock *po, void *frame, int status)
static int __packet_get_status(struct packet_sock *po, void *frame)
{
union tpacket_uhdr h;
-
- smp_rmb();
+ int status;

h.raw = frame;
switch (po->tp_version) {
case TPACKET_V1:
flush_dcache_page(pgv_to_page(&h.h1->tp_status));
- return h.h1->tp_status;
+ status = h.h1->tp_status;
+ break;
case TPACKET_V2:
flush_dcache_page(pgv_to_page(&h.h2->tp_status));
- return h.h2->tp_status;
+ status = h.h2->tp_status;
+ break;
case TPACKET_V3:
flush_dcache_page(pgv_to_page(&h.h3->tp_status));
- return h.h3->tp_status;
+ status = h.h3->tp_status;
+ break;
default:
WARN(1, "TPACKET version not supported.\n");
BUG();
- return 0;
+ status = 0;
+ break;
}
+ smp_rmb();
+ return status;
}

static __u32 tpacket_get_timestamp(struct sk_buff *skb, struct timespec *ts,
--
2.17.1
Sorry for a lack of information- it is the first my patch.

The opposie smp_wmb() is in: https://elixir.bootlin.com/linux/v4.15/source/net/packet/af_packet.c#L415

On my eye smp_rmb() should be moved *after* actual read of status to be ensured that no read after __packet_get_status() happens before actual read of status.

Note, that tpacket_snd() polls a ring [https://elixir.bootlin.com/linux/v4.15/source/net/packet/af_packet.c#L2677], possibly waiting for a published packet (by published I mean a packet with a TP_STATUS_SEND_REQUEST status).

It is important to have a guarantee that no load was reordered before we know the status, isn't it?

Please note that it is possible that I am wrong, so please take this into account. However, if I am wrong please let me know.

From the other side, what is the point to place smp_rmb() before read of status?

\
 
 \ /
  Last update: 2018-07-08 11:29    [W:0.036 / U:3.148 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site