[v2,06/15] unix: Add sender_ino of packets and socket

Submitted by Kirill Tkhai on May 27, 2016, 1:06 p.m.

Details

Message ID 146435439802.31234.1390196819288666479.stgit@pro
State Rejected
Series "Support for packet's msg_name in receive queue of promiscous DGRAM sockets"
Headers show

Commit Message

Kirill Tkhai May 27, 2016, 1:06 p.m.
Add optional field "sender_ino" to packet and socket proto.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 images/sk-packet.proto |    1 +
 images/sk-unix.proto   |    2 ++
 2 files changed, 3 insertions(+)

Patch hide | download patch | download mbox

diff --git a/images/sk-packet.proto b/images/sk-packet.proto
index 10ef5c9..2e1c8b3 100644
--- a/images/sk-packet.proto
+++ b/images/sk-packet.proto
@@ -1,4 +1,5 @@ 
 message sk_packet_entry {
 	required uint32		id_for		= 1;
 	required uint32		length		= 2;
+	optional uint32		sender_ino	= 3;
 }
diff --git a/images/sk-unix.proto b/images/sk-unix.proto
index aa2bcf7..7b71ebf 100644
--- a/images/sk-unix.proto
+++ b/images/sk-unix.proto
@@ -45,4 +45,6 @@  message unix_sk_entry {
 	 * Relative socket name may have prefix.
 	 */
 	optional string			name_dir	= 14;
+
+	optional uint32			sender_ino	= 15;
 }

Comments

Pavel Emelianov May 30, 2016, 11:40 a.m.
On 05/27/2016 04:06 PM, Kirill Tkhai wrote:
> Add optional field "sender_ino" to packet and socket proto.
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  images/sk-packet.proto |    1 +
>  images/sk-unix.proto   |    2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/images/sk-packet.proto b/images/sk-packet.proto
> index 10ef5c9..2e1c8b3 100644
> --- a/images/sk-packet.proto
> +++ b/images/sk-packet.proto
> @@ -1,4 +1,5 @@
>  message sk_packet_entry {
>  	required uint32		id_for		= 1;
>  	required uint32		length		= 2;
> +	optional uint32		sender_ino	= 3;

This is understood.

>  }
> diff --git a/images/sk-unix.proto b/images/sk-unix.proto
> index aa2bcf7..7b71ebf 100644
> --- a/images/sk-unix.proto
> +++ b/images/sk-unix.proto
> @@ -45,4 +45,6 @@ message unix_sk_entry {
>  	 * Relative socket name may have prefix.
>  	 */
>  	optional string			name_dir	= 14;
> +
> +	optional uint32			sender_ino	= 15;

Why do you need this new number on socket entry?

>  }
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
> .
>
Kirill Tkhai May 30, 2016, 12:30 p.m.
On 30.05.2016 14:40, Pavel Emelyanov wrote:
> On 05/27/2016 04:06 PM, Kirill Tkhai wrote:
>> Add optional field "sender_ino" to packet and socket proto.
>>
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>> ---
>>  images/sk-packet.proto |    1 +
>>  images/sk-unix.proto   |    2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/images/sk-packet.proto b/images/sk-packet.proto
>> index 10ef5c9..2e1c8b3 100644
>> --- a/images/sk-packet.proto
>> +++ b/images/sk-packet.proto
>> @@ -1,4 +1,5 @@
>>  message sk_packet_entry {
>>  	required uint32		id_for		= 1;
>>  	required uint32		length		= 2;
>> +	optional uint32		sender_ino	= 3;
> 
> This is understood.
> 
>>  }
>> diff --git a/images/sk-unix.proto b/images/sk-unix.proto
>> index aa2bcf7..7b71ebf 100644
>> --- a/images/sk-unix.proto
>> +++ b/images/sk-unix.proto
>> @@ -45,4 +45,6 @@ message unix_sk_entry {
>>  	 * Relative socket name may have prefix.
>>  	 */
>>  	optional string			name_dir	= 14;
>> +
>> +	optional uint32			sender_ino	= 15;
> 
> Why do you need this new number on socket entry?

This allows to understand that a DGRAM socket is not a promiscuous.
If so, we may set a sender as our queuer (see sk_queuer()).

Otherwise, we would have to do additional work on restore. We would
have to scan skb queue to understand if it's promiscuous.

Using unix_sk_entry::sender_ino, we easily mark the most sockets as
"having queuer", and skip them on "resolve senders" stage.
See resolve_unix_peers() changes.

>>  }
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU@openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu
>> .
>>
>
Pavel Emelianov May 31, 2016, 11:02 a.m.
On 05/30/2016 03:30 PM, Kirill Tkhai wrote:
> 
> 
> On 30.05.2016 14:40, Pavel Emelyanov wrote:
>> On 05/27/2016 04:06 PM, Kirill Tkhai wrote:
>>> Add optional field "sender_ino" to packet and socket proto.
>>>
>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>> ---
>>>  images/sk-packet.proto |    1 +
>>>  images/sk-unix.proto   |    2 ++
>>>  2 files changed, 3 insertions(+)
>>>
>>> diff --git a/images/sk-packet.proto b/images/sk-packet.proto
>>> index 10ef5c9..2e1c8b3 100644
>>> --- a/images/sk-packet.proto
>>> +++ b/images/sk-packet.proto
>>> @@ -1,4 +1,5 @@
>>>  message sk_packet_entry {
>>>  	required uint32		id_for		= 1;
>>>  	required uint32		length		= 2;
>>> +	optional uint32		sender_ino	= 3;
>>
>> This is understood.
>>
>>>  }
>>> diff --git a/images/sk-unix.proto b/images/sk-unix.proto
>>> index aa2bcf7..7b71ebf 100644
>>> --- a/images/sk-unix.proto
>>> +++ b/images/sk-unix.proto
>>> @@ -45,4 +45,6 @@ message unix_sk_entry {
>>>  	 * Relative socket name may have prefix.
>>>  	 */
>>>  	optional string			name_dir	= 14;
>>> +
>>> +	optional uint32			sender_ino	= 15;
>>
>> Why do you need this new number on socket entry?
> 
> This allows to understand that a DGRAM socket is not a promiscuous.
> If so, we may set a sender as our queuer (see sk_queuer()).
> 
> Otherwise, we would have to do additional work on restore. We would
> have to scan skb queue to understand if it's promiscuous.
> 
> Using unix_sk_entry::sender_ino, we easily mark the most sockets as
> "having queuer", and skip them on "resolve senders" stage.
> See resolve_unix_peers() changes.

OK. From my perspective you don't need this in image. During resolve_unix_peers()
we scan all the sockets, you can add analysis of the packets and find out
whether the socket is no-queuer or not.
Kirill Tkhai May 31, 2016, 1:03 p.m.
On 31.05.2016 14:02, Pavel Emelyanov wrote:
> On 05/30/2016 03:30 PM, Kirill Tkhai wrote:
>>
>>
>> On 30.05.2016 14:40, Pavel Emelyanov wrote:
>>> On 05/27/2016 04:06 PM, Kirill Tkhai wrote:
>>>> Add optional field "sender_ino" to packet and socket proto.
>>>>
>>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>>> ---
>>>>  images/sk-packet.proto |    1 +
>>>>  images/sk-unix.proto   |    2 ++
>>>>  2 files changed, 3 insertions(+)
>>>>
>>>> diff --git a/images/sk-packet.proto b/images/sk-packet.proto
>>>> index 10ef5c9..2e1c8b3 100644
>>>> --- a/images/sk-packet.proto
>>>> +++ b/images/sk-packet.proto
>>>> @@ -1,4 +1,5 @@
>>>>  message sk_packet_entry {
>>>>  	required uint32		id_for		= 1;
>>>>  	required uint32		length		= 2;
>>>> +	optional uint32		sender_ino	= 3;
>>>
>>> This is understood.
>>>
>>>>  }
>>>> diff --git a/images/sk-unix.proto b/images/sk-unix.proto
>>>> index aa2bcf7..7b71ebf 100644
>>>> --- a/images/sk-unix.proto
>>>> +++ b/images/sk-unix.proto
>>>> @@ -45,4 +45,6 @@ message unix_sk_entry {
>>>>  	 * Relative socket name may have prefix.
>>>>  	 */
>>>>  	optional string			name_dir	= 14;
>>>> +
>>>> +	optional uint32			sender_ino	= 15;
>>>
>>> Why do you need this new number on socket entry?
>>
>> This allows to understand that a DGRAM socket is not a promiscuous.
>> If so, we may set a sender as our queuer (see sk_queuer()).
>>
>> Otherwise, we would have to do additional work on restore. We would
>> have to scan skb queue to understand if it's promiscuous.
>>
>> Using unix_sk_entry::sender_ino, we easily mark the most sockets as
>> "having queuer", and skip them on "resolve senders" stage.
>> See resolve_unix_peers() changes.
> 
> OK. From my perspective you don't need this in image. During resolve_unix_peers()
> we scan all the sockets, you can add analysis of the packets and find out
> whether the socket is no-queuer or not.

I think this trick may speed-up restore in some way. If you are worrying about
using of space for that, I can add USK_HAS_SENDER flag instead. Is it OK for you?
Pavel Emelianov June 1, 2016, 10:45 a.m.
On 05/31/2016 04:03 PM, Kirill Tkhai wrote:
> 
> 
> On 31.05.2016 14:02, Pavel Emelyanov wrote:
>> On 05/30/2016 03:30 PM, Kirill Tkhai wrote:
>>>
>>>
>>> On 30.05.2016 14:40, Pavel Emelyanov wrote:
>>>> On 05/27/2016 04:06 PM, Kirill Tkhai wrote:
>>>>> Add optional field "sender_ino" to packet and socket proto.
>>>>>
>>>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>>>> ---
>>>>>  images/sk-packet.proto |    1 +
>>>>>  images/sk-unix.proto   |    2 ++
>>>>>  2 files changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/images/sk-packet.proto b/images/sk-packet.proto
>>>>> index 10ef5c9..2e1c8b3 100644
>>>>> --- a/images/sk-packet.proto
>>>>> +++ b/images/sk-packet.proto
>>>>> @@ -1,4 +1,5 @@
>>>>>  message sk_packet_entry {
>>>>>  	required uint32		id_for		= 1;
>>>>>  	required uint32		length		= 2;
>>>>> +	optional uint32		sender_ino	= 3;
>>>>
>>>> This is understood.
>>>>
>>>>>  }
>>>>> diff --git a/images/sk-unix.proto b/images/sk-unix.proto
>>>>> index aa2bcf7..7b71ebf 100644
>>>>> --- a/images/sk-unix.proto
>>>>> +++ b/images/sk-unix.proto
>>>>> @@ -45,4 +45,6 @@ message unix_sk_entry {
>>>>>  	 * Relative socket name may have prefix.
>>>>>  	 */
>>>>>  	optional string			name_dir	= 14;
>>>>> +
>>>>> +	optional uint32			sender_ino	= 15;
>>>>
>>>> Why do you need this new number on socket entry?
>>>
>>> This allows to understand that a DGRAM socket is not a promiscuous.
>>> If so, we may set a sender as our queuer (see sk_queuer()).
>>>
>>> Otherwise, we would have to do additional work on restore. We would
>>> have to scan skb queue to understand if it's promiscuous.
>>>
>>> Using unix_sk_entry::sender_ino, we easily mark the most sockets as
>>> "having queuer", and skip them on "resolve senders" stage.
>>> See resolve_unix_peers() changes.
>>
>> OK. From my perspective you don't need this in image. During resolve_unix_peers()
>> we scan all the sockets, you can add analysis of the packets and find out
>> whether the socket is no-queuer or not.
> 
> I think this trick may speed-up restore in some way. If you are worrying about
> using of space for that, I can add USK_HAS_SENDER flag instead. Is it OK for you?

No, please. The information in sk-packet.img is enough to decide what
kind of sender restore we have. We pull in this image into memory anyway.

-- Pavel
Kirill Tkhai June 1, 2016, 10:51 a.m.
On 01.06.2016 13:45, Pavel Emelyanov wrote:
> On 05/31/2016 04:03 PM, Kirill Tkhai wrote:
>>
>>
>> On 31.05.2016 14:02, Pavel Emelyanov wrote:
>>> On 05/30/2016 03:30 PM, Kirill Tkhai wrote:
>>>>
>>>>
>>>> On 30.05.2016 14:40, Pavel Emelyanov wrote:
>>>>> On 05/27/2016 04:06 PM, Kirill Tkhai wrote:
>>>>>> Add optional field "sender_ino" to packet and socket proto.
>>>>>>
>>>>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>>>>> ---
>>>>>>  images/sk-packet.proto |    1 +
>>>>>>  images/sk-unix.proto   |    2 ++
>>>>>>  2 files changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/images/sk-packet.proto b/images/sk-packet.proto
>>>>>> index 10ef5c9..2e1c8b3 100644
>>>>>> --- a/images/sk-packet.proto
>>>>>> +++ b/images/sk-packet.proto
>>>>>> @@ -1,4 +1,5 @@
>>>>>>  message sk_packet_entry {
>>>>>>  	required uint32		id_for		= 1;
>>>>>>  	required uint32		length		= 2;
>>>>>> +	optional uint32		sender_ino	= 3;
>>>>>
>>>>> This is understood.
>>>>>
>>>>>>  }
>>>>>> diff --git a/images/sk-unix.proto b/images/sk-unix.proto
>>>>>> index aa2bcf7..7b71ebf 100644
>>>>>> --- a/images/sk-unix.proto
>>>>>> +++ b/images/sk-unix.proto
>>>>>> @@ -45,4 +45,6 @@ message unix_sk_entry {
>>>>>>  	 * Relative socket name may have prefix.
>>>>>>  	 */
>>>>>>  	optional string			name_dir	= 14;
>>>>>> +
>>>>>> +	optional uint32			sender_ino	= 15;
>>>>>
>>>>> Why do you need this new number on socket entry?
>>>>
>>>> This allows to understand that a DGRAM socket is not a promiscuous.
>>>> If so, we may set a sender as our queuer (see sk_queuer()).
>>>>
>>>> Otherwise, we would have to do additional work on restore. We would
>>>> have to scan skb queue to understand if it's promiscuous.
>>>>
>>>> Using unix_sk_entry::sender_ino, we easily mark the most sockets as
>>>> "having queuer", and skip them on "resolve senders" stage.
>>>> See resolve_unix_peers() changes.
>>>
>>> OK. From my perspective you don't need this in image. During resolve_unix_peers()
>>> we scan all the sockets, you can add analysis of the packets and find out
>>> whether the socket is no-queuer or not.
>>
>> I think this trick may speed-up restore in some way. If you are worrying about
>> using of space for that, I can add USK_HAS_SENDER flag instead. Is it OK for you?
> 
> No, please. The information in sk-packet.img is enough to decide what
> kind of sender restore we have. We pull in this image into memory anyway.

But if we decide this on restore, it will require [nr sockers] x [nr packers] comparations.
Pavel Emelianov June 1, 2016, 2:39 p.m.
On 06/01/2016 01:51 PM, Kirill Tkhai wrote:
> On 01.06.2016 13:45, Pavel Emelyanov wrote:
>> On 05/31/2016 04:03 PM, Kirill Tkhai wrote:
>>>
>>>
>>> On 31.05.2016 14:02, Pavel Emelyanov wrote:
>>>> On 05/30/2016 03:30 PM, Kirill Tkhai wrote:
>>>>>
>>>>>
>>>>> On 30.05.2016 14:40, Pavel Emelyanov wrote:
>>>>>> On 05/27/2016 04:06 PM, Kirill Tkhai wrote:
>>>>>>> Add optional field "sender_ino" to packet and socket proto.
>>>>>>>
>>>>>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>>>>>> ---
>>>>>>>  images/sk-packet.proto |    1 +
>>>>>>>  images/sk-unix.proto   |    2 ++
>>>>>>>  2 files changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/images/sk-packet.proto b/images/sk-packet.proto
>>>>>>> index 10ef5c9..2e1c8b3 100644
>>>>>>> --- a/images/sk-packet.proto
>>>>>>> +++ b/images/sk-packet.proto
>>>>>>> @@ -1,4 +1,5 @@
>>>>>>>  message sk_packet_entry {
>>>>>>>  	required uint32		id_for		= 1;
>>>>>>>  	required uint32		length		= 2;
>>>>>>> +	optional uint32		sender_ino	= 3;
>>>>>>
>>>>>> This is understood.
>>>>>>
>>>>>>>  }
>>>>>>> diff --git a/images/sk-unix.proto b/images/sk-unix.proto
>>>>>>> index aa2bcf7..7b71ebf 100644
>>>>>>> --- a/images/sk-unix.proto
>>>>>>> +++ b/images/sk-unix.proto
>>>>>>> @@ -45,4 +45,6 @@ message unix_sk_entry {
>>>>>>>  	 * Relative socket name may have prefix.
>>>>>>>  	 */
>>>>>>>  	optional string			name_dir	= 14;
>>>>>>> +
>>>>>>> +	optional uint32			sender_ino	= 15;
>>>>>>
>>>>>> Why do you need this new number on socket entry?
>>>>>
>>>>> This allows to understand that a DGRAM socket is not a promiscuous.
>>>>> If so, we may set a sender as our queuer (see sk_queuer()).
>>>>>
>>>>> Otherwise, we would have to do additional work on restore. We would
>>>>> have to scan skb queue to understand if it's promiscuous.
>>>>>
>>>>> Using unix_sk_entry::sender_ino, we easily mark the most sockets as
>>>>> "having queuer", and skip them on "resolve senders" stage.
>>>>> See resolve_unix_peers() changes.
>>>>
>>>> OK. From my perspective you don't need this in image. During resolve_unix_peers()
>>>> we scan all the sockets, you can add analysis of the packets and find out
>>>> whether the socket is no-queuer or not.
>>>
>>> I think this trick may speed-up restore in some way. If you are worrying about
>>> using of space for that, I can add USK_HAS_SENDER flag instead. Is it OK for you?
>>
>> No, please. The information in sk-packet.img is enough to decide what
>> kind of sender restore we have. We pull in this image into memory anyway.
> 
> But if we decide this on restore, it will require [nr sockers] x [nr packers] comparations.

Don't we do the same on dump side? You have sockets and packets in them,
you have to find out whether each socket has zero, one or many queuers.
What's the difference when doing it?
Kirill Tkhai June 1, 2016, 3:10 p.m.
On 01.06.2016 17:39, Pavel Emelyanov wrote:
> On 06/01/2016 01:51 PM, Kirill Tkhai wrote:
>> On 01.06.2016 13:45, Pavel Emelyanov wrote:
>>> On 05/31/2016 04:03 PM, Kirill Tkhai wrote:
>>>>
>>>>
>>>> On 31.05.2016 14:02, Pavel Emelyanov wrote:
>>>>> On 05/30/2016 03:30 PM, Kirill Tkhai wrote:
>>>>>>
>>>>>>
>>>>>> On 30.05.2016 14:40, Pavel Emelyanov wrote:
>>>>>>> On 05/27/2016 04:06 PM, Kirill Tkhai wrote:
>>>>>>>> Add optional field "sender_ino" to packet and socket proto.
>>>>>>>>
>>>>>>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>>>>>>>> ---
>>>>>>>>  images/sk-packet.proto |    1 +
>>>>>>>>  images/sk-unix.proto   |    2 ++
>>>>>>>>  2 files changed, 3 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/images/sk-packet.proto b/images/sk-packet.proto
>>>>>>>> index 10ef5c9..2e1c8b3 100644
>>>>>>>> --- a/images/sk-packet.proto
>>>>>>>> +++ b/images/sk-packet.proto
>>>>>>>> @@ -1,4 +1,5 @@
>>>>>>>>  message sk_packet_entry {
>>>>>>>>  	required uint32		id_for		= 1;
>>>>>>>>  	required uint32		length		= 2;
>>>>>>>> +	optional uint32		sender_ino	= 3;
>>>>>>>
>>>>>>> This is understood.
>>>>>>>
>>>>>>>>  }
>>>>>>>> diff --git a/images/sk-unix.proto b/images/sk-unix.proto
>>>>>>>> index aa2bcf7..7b71ebf 100644
>>>>>>>> --- a/images/sk-unix.proto
>>>>>>>> +++ b/images/sk-unix.proto
>>>>>>>> @@ -45,4 +45,6 @@ message unix_sk_entry {
>>>>>>>>  	 * Relative socket name may have prefix.
>>>>>>>>  	 */
>>>>>>>>  	optional string			name_dir	= 14;
>>>>>>>> +
>>>>>>>> +	optional uint32			sender_ino	= 15;
>>>>>>>
>>>>>>> Why do you need this new number on socket entry?
>>>>>>
>>>>>> This allows to understand that a DGRAM socket is not a promiscuous.
>>>>>> If so, we may set a sender as our queuer (see sk_queuer()).
>>>>>>
>>>>>> Otherwise, we would have to do additional work on restore. We would
>>>>>> have to scan skb queue to understand if it's promiscuous.
>>>>>>
>>>>>> Using unix_sk_entry::sender_ino, we easily mark the most sockets as
>>>>>> "having queuer", and skip them on "resolve senders" stage.
>>>>>> See resolve_unix_peers() changes.
>>>>>
>>>>> OK. From my perspective you don't need this in image. During resolve_unix_peers()
>>>>> we scan all the sockets, you can add analysis of the packets and find out
>>>>> whether the socket is no-queuer or not.
>>>>
>>>> I think this trick may speed-up restore in some way. If you are worrying about
>>>> using of space for that, I can add USK_HAS_SENDER flag instead. Is it OK for you?
>>>
>>> No, please. The information in sk-packet.img is enough to decide what
>>> kind of sender restore we have. We pull in this image into memory anyway.
>>
>> But if we decide this on restore, it will require [nr sockers] x [nr packers] comparations.
> 
> Don't we do the same on dump side? You have sockets and packets in them,
> you have to find out whether each socket has zero, one or many queuers.
> What's the difference when doing it?

On dump we receive socket's packets and simply compare cur_pkt_ino != prev_pkt_ino.
No iteration is there. So the difference presents.

Anyway, if you think it's not big time wasted here, I do a version without precalculated sender.