On 5/18/26 4:00 PM, Eelco Chaudron wrote:
>
>
> On 6 May 2026, at 0:52, Ilya Maximets wrote:
>
>> Windows support was deprecated in 3.7, it's time to remove it.
>>
>> We already removed the installer and the datapath implementation.
>> Now removing all the code related to talking to the Windows datapath,
>> building on Windows, using MSVC as a compiler, and the final bits of
>> documentation related to Windows-specific behavior of certain features.
>>
>> It is possible to split this change into smaller parts, but it will
>> be a non-trivial amount of work to keep the patches in a working state
>> in the process. So, just removing everything at once. It would also
>> be easier to re-apply in case someone wants to resurrect this code or
>> maintain the Windows support in their own fork.
>>
>> A couple mentions of MSVC quirks are left in public headers as the
>> code is good enough and there is no real need to change the way it
>> is written at the moment.
>>
>> Signed-off-by: Ilya Maximets <[email protected]>
>
> This took some time to review, so it was probably also hard to make all these
> changes ;)
Yeah, it took some time to write for sure. :)
>
> See some comments inline, but also some general ones here:
>
> - faq/contributing.rst still has the below paragraph:
>
> The Windows datapath kernel module support, on the other hand, is
> maintained within the OVS tree, so patches for that can go directly to
> ovs-dev.
I thought I removed this, but I guess it somehow came back.
Will clean this up.
>
> - /faq/general.rst still has the following Windows reference:
>
> We've seen
> Open vSwitch ported to a number of different platforms, including FreeBSD,
> Windows, and even non-POSIX embedded systems.
I left this one intentionally, because we have definitely seen the Windows port.
It doesn't mean it's supported in the mainline. The same as the mentioned
non-POSIX embedded systems.
>
> - The below comments still reference to windows, but I guess it should be
> fine:
>
> - vswitchd/bridge.c:2125; "only necessary on Windows"
I left this one because it still seems a little weird to allow slashes in
names, even if it's not necessary on other platforms. The comment says that
"it's no great loss elsewhere."
> - include/openvswitch/util.h:146,285; MSVC variadic/init macro comments
Left these ones as it's a header and there is no much point in rewriting these
macros. But it's good to keep the mentioning of why they were written this
way in the first place, in case someone wants to re-do them in the future.
> - include/openvswitch/ofp-errors.h:44; "and Windows"
This talks about operating systems in general, so it seems fine to leave as is.
> - lib/timeval.c:775; Visual Studio 2013 crash comment
Similar to util.h, I didn't feel like it is necessary to change the code, the
check seems reasonable, but the comment explains why things are the way they
are.
>
> //Eelco
>
> [...]
>
>> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
>> index f22a87934..65a9c3be5 100644
>> --- a/lib/dpif-netlink.c
>> +++ b/lib/dpif-netlink.c
>
> [...]
>
>> @@ -3555,8 +3249,6 @@ dpif_netlink_ct_timeout_policy_dump_done(struct dpif
>> *dpif OVS_UNUSED,
>> free(dump_state);
>> return err;
>> }
>> -#endif
>> -
>
> The diff does not show it, but we should keep one additional
> newline after the FF.
I guess, you meant "before"? The FF was after the endif and the empty line
in the original code, AFAICT.
>
> [...]
>
>> diff --git a/lib/lockfile.c b/lib/lockfile.c
>> index 42782d29e..6ecbb307e 100644
>> --- a/lib/lockfile.c
>> +++ b/lib/lockfile.c
>
> In the struct below, there is still 'HANDLE lock_handle', should this be
> removed?
Yep.
>
> struct lockfile {
> struct hmap_node hmap_node;
> char *name;
> dev_t device;
> ino_t inode;
> int fd;
> HANDLE lock_handle;
> };
>
> [...]
>
>> diff --git a/lib/poll-loop.c b/lib/poll-loop.c
>> index 70fabeb8a..cbeb6990b 100644
>> --- a/lib/poll-loop.c
>> +++ b/lib/poll-loop.c
>
> The below structure and some of the other code in poll-loop.c still have
> 'wevent' references, I guess they all need to be removed.
Yep. Missed those.
>
> struct poll_node {
> struct hmap_node hmap_node;
> struct pollfd pollfd; /* Events to pass to time_poll(). */
> HANDLE wevent; /* Events for WaitForMultipleObjects(). */
> const char *where; /* Where poll_node was created. */
> };
>
> [...]
>
>
>> diff --git a/ovsdb/log.c b/ovsdb/log.c
>> index 86d6bdc24..6d75427cf 100644
>> --- a/ovsdb/log.c
>> +++ b/ovsdb/log.c
>>
>
> [...]
>
>> @@ -882,20 +822,12 @@ void
>> ovsdb_log_replace_abort(struct ovsdb_log *new)
>> {
>> if (new) {
>> - /* Unlink the new file, but only after we close it (because Windows
>> - * does not allow removing an open file). */
>> - char *name = xstrdup(new->name);
>> + /* Unlink and close the new file. */
>> + unlink(new->name);
>> ovsdb_log_close(new);
>> - unlink(name);
>> - free(name);
>> }
>> }
>>
>> -void
>> -ovsdb_log_disable_renaming_open_files(void)
>
> This function's declaration still exists in ./ovsdb/log.h.
Ack.
>
>> -{
>> - rename_open_files = false;
>> -}
>>
> [...]
>
>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>> index 9a10b78a9..2f7123586 100755
>> --- a/utilities/checkpatch.py
>> +++ b/utilities/checkpatch.py
>> @@ -1059,7 +1059,7 @@ def ovs_checkpatch_parse(text, filename, author=None,
>> committer=None):
>> continue
>>
>> # Skip files which have /datapath in them, since they are
>> - # linux or windows coding standards
>> + # linux coding standards
>
> Should we add a dot (.) while we are at it?
Sure.
I suppose, reviewing the diff here is much easier than reviewing a v2.
I can fold the following changes into appropriate patches while applying
the set, if they look good to you:
diff --git a/Documentation/faq/contributing.rst
b/Documentation/faq/contributing.rst
index 9f7ea121d..1790a3c20 100644
--- a/Documentation/faq/contributing.rst
+++ b/Documentation/faq/contributing.rst
@@ -65,9 +65,6 @@ Q: How do I add support for a new field or header?
without it. If you implement kernel module support for Linux, then the
Linux kernel "netdev" mailing list is the place to submit that support
first; please read up on the Linux kernel development process separately.
- The Windows datapath kernel module support, on the other hand, is
- maintained within the OVS tree, so patches for that can go directly to
- ovs-dev.
Q: How do I add support for a new OpenFlow action?
diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 1f7313f13..b62bb4ed5 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -50,7 +50,7 @@ Q: What Linux kernel versions does each Open vSwitch release
work with?
Q: Does Open vSwitch support running on Windows (Hyper-V)?
A: Support for the Windows datapath, a.k.a. Hyper-V, was deprecated starting
- with Open vSwitch 3.7 and the source code was completely removed from the
+ with Open vSwitch 3.7, and the source code was completely removed from the
Open vSwitch source tree in the next release.
Q: Are all features available with all datapaths?
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 65a9c3be5..2bf7e7cf2 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -3249,6 +3249,7 @@ dpif_netlink_ct_timeout_policy_dump_done(struct dpif
*dpif OVS_UNUSED,
free(dump_state);
return err;
}
+
/* Meters */
diff --git a/lib/lockfile.c b/lib/lockfile.c
index 6ecbb307e..3ad5f0a5d 100644
--- a/lib/lockfile.c
+++ b/lib/lockfile.c
@@ -44,7 +44,6 @@ struct lockfile {
dev_t device;
ino_t inode;
int fd;
- HANDLE lock_handle;
};
/* Lock table.
diff --git a/lib/poll-loop.c b/lib/poll-loop.c
index cbeb6990b..a5fc4baf0 100644
--- a/lib/poll-loop.c
+++ b/lib/poll-loop.c
@@ -41,7 +41,6 @@ COVERAGE_DEFINE(poll_zero_timeout);
struct poll_node {
struct hmap_node hmap_node;
struct pollfd pollfd; /* Events to pass to time_poll(). */
- HANDLE wevent; /* Events for WaitForMultipleObjects(). */
const char *where; /* Where poll_node was created. */
};
@@ -57,20 +56,15 @@ struct poll_loop {
static struct poll_loop *poll_loop(void);
-/* Look up the node with same fd or wevent. */
+/* Look up the node with same fd. */
static struct poll_node *
-find_poll_node(struct poll_loop *loop, int fd, HANDLE wevent)
+find_poll_node(struct poll_loop *loop, int fd)
{
struct poll_node *node;
- /* Both 'fd' and 'wevent' cannot be set. */
- ovs_assert(!fd != !wevent);
-
- HMAP_FOR_EACH_WITH_HASH (node, hmap_node,
- hash_2words(fd, (uint32_t)wevent),
+ HMAP_FOR_EACH_WITH_HASH (node, hmap_node, hash_int(fd, 0),
&loop->poll_nodes) {
- if ((fd && node->pollfd.fd == fd)
- || (wevent && node->wevent == wevent)) {
+ if (node->pollfd.fd == fd) {
return node;
}
}
@@ -90,27 +84,22 @@ find_poll_node(struct poll_loop *loop, int fd, HANDLE
wevent)
* automatically provide the caller's source file and line number for
* 'where'.) */
static void
-poll_create_node(int fd, HANDLE wevent, short int events, const char *where)
+poll_create_node(int fd, short int events, const char *where)
{
struct poll_loop *loop = poll_loop();
struct poll_node *node;
COVERAGE_INC(poll_create_node);
- /* Both 'fd' and 'wevent' cannot be set. */
- ovs_assert(!fd != !wevent);
-
/* Check for duplicate. If found, "or" the events. */
- node = find_poll_node(loop, fd, wevent);
+ node = find_poll_node(loop, fd);
if (node) {
node->pollfd.events |= events;
} else {
node = xzalloc(sizeof *node);
- hmap_insert(&loop->poll_nodes, &node->hmap_node,
- hash_2words(fd, (uint32_t)wevent));
+ hmap_insert(&loop->poll_nodes, &node->hmap_node, hash_int(fd, 0));
node->pollfd.fd = fd;
node->pollfd.events = events;
- node->wevent = wevent;
node->where = where;
}
}
@@ -129,7 +118,7 @@ poll_create_node(int fd, HANDLE wevent, short int events,
const char *where)
void
poll_fd_wait_at(int fd, short int events, const char *where)
{
- poll_create_node(fd, 0, events, where);
+ poll_create_node(fd, events, where);
}
/* Causes the following call to poll_block() to block for no more than 'msec'
@@ -280,7 +269,6 @@ poll_block(void)
struct poll_loop *loop = poll_loop();
struct poll_node *node;
struct pollfd *pollfds;
- HANDLE *wevents = NULL;
int elapsed;
int retval;
int i;
@@ -302,7 +290,7 @@ poll_block(void)
pollfds[i++] = node->pollfd;
}
- retval = time_poll(pollfds, hmap_count(&loop->poll_nodes), wevents,
+ retval = time_poll(pollfds, hmap_count(&loop->poll_nodes),
loop->timeout_when, &elapsed);
if (retval < 0) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -323,7 +311,6 @@ poll_block(void)
loop->timeout_when = LLONG_MAX;
loop->timeout_where = NULL;
free(pollfds);
- free(wevents);
/* Handle any pending signals before doing anything else. */
fatal_signal_run();
diff --git a/lib/timeval.c b/lib/timeval.c
index e1fc35b27..5d7e30b2a 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -282,7 +282,7 @@ time_alarm(unsigned int secs)
*
* Stores the number of milliseconds elapsed during poll in '*elapsed'. */
int
-time_poll(struct pollfd *pollfds, int n_pollfds, HANDLE *handles OVS_UNUSED,
+time_poll(struct pollfd *pollfds, int n_pollfds,
long long int timeout_when, int *elapsed)
{
long long int *last_wakeup = last_wakeup_get();
diff --git a/lib/timeval.h b/lib/timeval.h
index 422db8c63..9d1ebda29 100644
--- a/lib/timeval.h
+++ b/lib/timeval.h
@@ -54,7 +54,7 @@ long long int time_wall_usec(void);
void time_timespec(struct timespec *);
void time_wall_timespec(struct timespec *);
void time_alarm(unsigned int secs);
-int time_poll(struct pollfd *, int n_pollfds, HANDLE *handles,
+int time_poll(struct pollfd *, int n_pollfds,
long long int timeout_when, int *elapsed);
long long int timespec_to_msec(const struct timespec *);
diff --git a/lib/util.h b/lib/util.h
index e8884f4b2..03f0c151f 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -121,8 +121,6 @@ ovs_prefetch_range(const void *start, size_t size)
#define PRIxSIZE "zx"
#define PRIXSIZE "zX"
-typedef uint32_t HANDLE;
-
#ifdef __cplusplus
extern "C" {
#endif
diff --git a/ovsdb/log.h b/ovsdb/log.h
index 63e5681a0..735186101 100644
--- a/ovsdb/log.h
+++ b/ovsdb/log.h
@@ -97,7 +97,4 @@ struct ovsdb_error *ovsdb_log_replace_commit(struct ovsdb_log
*old,
OVS_WARN_UNUSED_RESULT;
void ovsdb_log_replace_abort(struct ovsdb_log *new);
-/* For testing. */
-void ovsdb_log_disable_renaming_open_files(void);
-
#endif /* ovsdb/log.h */
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 2f7123586..e46c28967 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -1059,7 +1059,7 @@ def ovs_checkpatch_parse(text, filename, author=None,
committer=None):
continue
# Skip files which have /datapath in them, since they are
- # linux coding standards
+ # linux coding standards.
if current_file.startswith('datapath'):
continue
if current_file.startswith('include/linux'):
---
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
