After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759490 - [review] platform: merge synchronous socket with event socket [th/platform-no-sync-socket-bgo759490]
[review] platform: merge synchronous socket with event socket [th/platform-no...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
1.0.x
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-12-15 12:57 UTC by Thomas Haller
Modified: 2015-12-17 17:47 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Thomas Haller 2015-12-15 12:57:46 UTC
Refactor platform again :)

- merge the synchrnous socket with the event socket. Now only one netlink socket is used, which potentially saves some re-fetching. Previously, we would use priv->nlh to perform an operation, and reload the object on priv->nlh_event.
We still need too many reloads, because often netlink events are just missing.

- reimplement nl_recvmsgs(). This gives more direct access to the message parsing and to the returned values.

- better track the pending request seq_number and match it with the answers. Previously, this was done very coarsely, only saying: we made a request for priv->nlh_seq_expect and we still require an answer to that.
Comment 1 Beniamino Galvani 2015-12-17 16:28:48 UTC
> platform: improve waiting for netlink response

+static void
+delayed_action_wait_for_nl_reponse_complete (NMPlatform *platform,

s/reponse/response/

+       out_seq_result = data->out_seq_result;
+
+       g_array_remove_index_fast (priv->delayed_action.list_wait_for_nl_response, idx);
+       data = NULL;

data is not used again below.

+       if (   NM_FLAGS_ANY (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_ALL)
+           && priv->delayed_action.is_handling < 5 /* avoid deep recursive stacks */) {

Probably I'm missing an obvious thing but how is it possible that
delayed_action_handle_all() gets called recursively?

Pushed some trivial fixups.
Comment 2 Thomas Haller 2015-12-17 17:29:13 UTC
(In reply to Beniamino Galvani from comment #1)

took your changes and fixed for your comments. Rebased all to master.


> Probably I'm missing an obvious thing but how is it possible that
> delayed_action_handle_all() gets called recursively?

I guess, it cannot really. I added a new commit that asserts against that.
Comment 3 Beniamino Galvani 2015-12-17 17:39:03 UTC
LGTM
Comment 4 Thomas Haller 2015-12-17 17:47:09 UTC
Thank you, Beniamino.


merged to master:

http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=5aba5685f2a7bf227ed923d969be29ce8d543b9a