Re: [systemd-devel] [PATCH v2] Add FDB support

2014-12-16 Thread Rauta, Alin
Hi Tom,

Thanks for your feedback. I will change VLAN to VLANId in the next patch. 
Regarding the flushing of entries that's done because we don't want to have 
dangling configuration between system restarts, but currently the discussion is 
ongoing internally and it may take some time. 

Best Regards,
Alin

-Original Message-
From: Tom Gundersen [mailto:t...@jklm.no] 
Sent: Monday, December 15, 2014 5:49 PM
To: Rauta, Alin
Cc: Lennart Poettering; systemd Mailing List; Kinsella, Ray
Subject: Re: [systemd-devel] [PATCH v2] Add FDB support

Hi Alin,

Thanks for the patch!

Overall it looks excellent, my only concern is with the clearing of FDB 
entries. Is there any reason to treat this differently than how we currently 
treat routes and addresses?

The current logic is that we don't support run-time reconfiguration (we only 
apply our (static) config once when the interfaces appear, and that's it). If 
there is some config there already we assume that it is intentional and leave 
it alone (i.e., if someone else have set addresses or routes we don't remove 
them, we just add new ones). In the future we plan to get a dbus API where 
networkd can be reconfigured at run-time (i.e., change which .network file is 
applied to a link), and then it definitely would make sense to flush routes and 
addresses when removing the .network from the link, but currently we don't do 
that at all.

If people want to reconfigure stuff now, they can of course just restart 
networkd, but then they should also manually flush the settings from the 
kernel, and this (changing the config, restarting the daemon and flushing) is 
more of a hack for testing than something we support.

So the question I have is: can we treat FDB entries the same as routes and 
addresses and simply leave them alone rather than flushing them, and then only 
add the flushing logic once we start supporting reconfiguration? If not, can 
you explain a bit more what the typical usecase is where flushing is desirable?

As to the naming, I like the suggestion made above to call it VLANId
rather than VLAN.

Cheers,

Tom

On Mon, Dec 15, 2014 at 11:20 AM, Alin Rauta alin.ra...@intel.com wrote:
 Hi,

 Based on your feedback, I've created a new patch for adding the FDB support.
 networkd takes ownership of the FDB table.

 Configuration example:

 cat /etc/systemd/network/em1.network

 [Match]
 Name=em1

 [Network]
 DHCP=v4

 [BridgeFDB]
 MACAddress=04:44:12:34:56:70
 VLAN=2

 [BridgeFDB]
 MACAddress=04:44:12:34:56:69
 VLAN=3

 Let me know what you think.

 Thanks,
 Alin

 Alin Rauta (1):
   Add FDB support

  Makefile.am  |   1 +
  man/systemd.network.xml  |  22 ++
  src/libsystemd/sd-rtnl/rtnl-message.c|  56 -
  src/libsystemd/sd-rtnl/rtnl-types.c  |  15 +-
  src/network/networkd-fdb.c   | 357 
 +++
  src/network/networkd-link.c  |  25 +++
  src/network/networkd-network-gperf.gperf |   2 +
  src/network/networkd-network.c   |  13 +-
  src/network/networkd.h   |  30 +++
  src/systemd/sd-rtnl.h|   4 +
  10 files changed, 514 insertions(+), 11 deletions(-)  create mode 
 100644 src/network/networkd-fdb.c

 --
 1.9.3

 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2] Add FDB support

2014-12-16 Thread Tom Gundersen
On Tue, Dec 16, 2014 at 4:28 PM, Rauta, Alin alin.ra...@intel.com wrote:
 Regarding the flushing of entries that's done because we don't want to have 
 dangling configuration between system restarts,

You mean these settings are remembered by the hardware between reboots?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2] Add FDB support

2014-12-16 Thread Rauta, Alin
Sorry,  between systemd restarts.
/Alin

-Original Message-
From: Tom Gundersen [mailto:t...@jklm.no] 
Sent: Tuesday, December 16, 2014 3:56 PM
To: Rauta, Alin
Cc: Lennart Poettering; systemd Mailing List; Kinsella, Ray
Subject: Re: [systemd-devel] [PATCH v2] Add FDB support

On Tue, Dec 16, 2014 at 4:28 PM, Rauta, Alin alin.ra...@intel.com wrote:
 Regarding the flushing of entries that's done because we don't want to have 
 dangling configuration between system restarts,

You mean these settings are remembered by the hardware between reboots?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2] Add FDB support

2014-12-16 Thread Tom Gundersen
On Tue, Dec 16, 2014 at 5:15 PM, Rauta, Alin alin.ra...@intel.com wrote:
 Sorry,  between systemd restarts.

Right. That's what I thought. In that case I would not worry about it
for now, as explained previously, and rather handle it together with
the flushing of addresses and routes when we get to that.

Cheers,

Tom

 -Original Message-
 From: Tom Gundersen [mailto:t...@jklm.no]
 Sent: Tuesday, December 16, 2014 3:56 PM
 To: Rauta, Alin
 Cc: Lennart Poettering; systemd Mailing List; Kinsella, Ray
 Subject: Re: [systemd-devel] [PATCH v2] Add FDB support

 On Tue, Dec 16, 2014 at 4:28 PM, Rauta, Alin alin.ra...@intel.com wrote:
 Regarding the flushing of entries that's done because we don't want to have 
 dangling configuration between system restarts,

 You mean these settings are remembered by the hardware between reboots?

 Cheers,

 Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH v2] Add FDB support

2014-12-15 Thread Alin Rauta
Hi,

Based on your feedback, I've created a new patch for adding the FDB support.
networkd takes ownership of the FDB table.

Configuration example:

 cat /etc/systemd/network/em1.network

[Match]
Name=em1

[Network]
DHCP=v4

[BridgeFDB]
MACAddress=04:44:12:34:56:70
VLAN=2

[BridgeFDB]
MACAddress=04:44:12:34:56:69
VLAN=3

Let me know what you think.

Thanks,
Alin

Alin Rauta (1):
  Add FDB support

 Makefile.am  |   1 +
 man/systemd.network.xml  |  22 ++
 src/libsystemd/sd-rtnl/rtnl-message.c|  56 -
 src/libsystemd/sd-rtnl/rtnl-types.c  |  15 +-
 src/network/networkd-fdb.c   | 357 +++
 src/network/networkd-link.c  |  25 +++
 src/network/networkd-network-gperf.gperf |   2 +
 src/network/networkd-network.c   |  13 +-
 src/network/networkd.h   |  30 +++
 src/systemd/sd-rtnl.h|   4 +
 10 files changed, 514 insertions(+), 11 deletions(-)
 create mode 100644 src/network/networkd-fdb.c

-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH v2] Add FDB support

2014-12-15 Thread Alin Rauta
Signed-off-by: Alin Rauta alin.ra...@intel.com
---
 Makefile.am  |   1 +
 man/systemd.network.xml  |  22 ++
 src/libsystemd/sd-rtnl/rtnl-message.c|  56 -
 src/libsystemd/sd-rtnl/rtnl-types.c  |  15 +-
 src/network/networkd-fdb.c   | 357 +++
 src/network/networkd-link.c  |  25 +++
 src/network/networkd-network-gperf.gperf |   2 +
 src/network/networkd-network.c   |  13 +-
 src/network/networkd.h   |  30 +++
 src/systemd/sd-rtnl.h|   4 +
 10 files changed, 514 insertions(+), 11 deletions(-)
 create mode 100644 src/network/networkd-fdb.c

diff --git a/Makefile.am b/Makefile.am
index 84b587d..226d298 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5271,6 +5271,7 @@ libsystemd_networkd_core_la_SOURCES = \
src/network/networkd-address.c \
src/network/networkd-route.c \
src/network/networkd-manager.c \
+   src/network/networkd-fdb.c \
src/network/networkd-address-pool.c
 
 nodist_libsystemd_networkd_core_la_SOURCES = \
diff --git a/man/systemd.network.xml b/man/systemd.network.xml
index 79c7a23..39bf385 100644
--- a/man/systemd.network.xml
+++ b/man/systemd.network.xml
@@ -549,6 +549,28 @@
 /refsect1
 
 refsect1
+title[BridgeFDB] Section Options/title
+paraThe literal[BridgeFDB]/literal section 
manages the forwarding database table of a port and accepts the following keys. 
Specify
+several literal[BridgeFDB]/literal sections to 
configure several static MAC table entries./para
+
+variablelist class='network-directives'
+varlistentry
+
termvarnameMACAddress=/varname/term
+listitem
+paraAs in the 
literal[Network]/literal section. This key is mandatory./para
+/listitem
+/varlistentry
+varlistentry
+termvarnameVLAN=/varname/term
+listitem
+paraThe VLAN for the new 
static MAC table entry.
+If omitted, no VLAN info is 
appended to the new static MAC table entry./para
+/listitem
+/varlistentry
+/variablelist
+/refsect1
+
+refsect1
 titleExample/title
 example
 title/etc/systemd/network/50-static.network/title
diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c 
b/src/libsystemd/sd-rtnl/rtnl-message.c
index 165e84d..9099440 100644
--- a/src/libsystemd/sd-rtnl/rtnl-message.c
+++ b/src/libsystemd/sd-rtnl/rtnl-message.c
@@ -220,6 +220,58 @@ int sd_rtnl_message_new_route(sd_rtnl *rtnl, 
sd_rtnl_message **ret,
 return 0;
 }
 
+int sd_rtnl_message_neigh_set_flags(sd_rtnl_message *m, uint8_t flags) {
+struct ndmsg *ndm;
+
+assert_return(m, -EINVAL);
+assert_return(m-hdr, -EINVAL);
+assert_return(rtnl_message_type_is_neigh(m-hdr-nlmsg_type), -EINVAL);
+
+ndm = NLMSG_DATA(m-hdr);
+ndm-ndm_flags |= flags;
+
+return 0;
+}
+
+int sd_rtnl_message_neigh_set_state(sd_rtnl_message *m, uint16_t state) {
+struct ndmsg *ndm;
+
+assert_return(m, -EINVAL);
+assert_return(m-hdr, -EINVAL);
+assert_return(rtnl_message_type_is_neigh(m-hdr-nlmsg_type), -EINVAL);
+
+ndm = NLMSG_DATA(m-hdr);
+ndm-ndm_state |= state;
+
+return 0;
+}
+
+int sd_rtnl_message_neigh_get_flags(sd_rtnl_message *m, uint8_t *flags) {
+struct ndmsg *ndm;
+
+assert_return(m, -EINVAL);
+assert_return(m-hdr, -EINVAL);
+assert_return(rtnl_message_type_is_neigh(m-hdr-nlmsg_type), -EINVAL);
+
+ndm = NLMSG_DATA(m-hdr);
+*flags = ndm-ndm_flags;
+
+return 0;
+}
+
+int sd_rtnl_message_neigh_get_state(sd_rtnl_message *m, uint16_t *state) {
+struct ndmsg *ndm;
+
+assert_return(m, -EINVAL);
+assert_return(m-hdr, -EINVAL);
+assert_return(rtnl_message_type_is_neigh(m-hdr-nlmsg_type), -EINVAL);
+
+ndm = NLMSG_DATA(m-hdr);
+*state = ndm-ndm_state;
+
+return 0;
+}
+
 int sd_rtnl_message_neigh_get_family(sd_rtnl_message *m, int *family) {
 struct ndmsg *ndm;
 
@@ -255,7 +307,9 @@ int sd_rtnl_message_new_neigh(sd_rtnl *rtnl, 
sd_rtnl_message **ret, uint16_t nlm
 int r;
 
 assert_return(rtnl_message_type_is_neigh(nlmsg_type), -EINVAL);
-assert_return(ndm_family == AF_INET || ndm_family == AF_INET6, 
-EINVAL);
+assert_return(ndm_family == 

Re: [systemd-devel] [PATCH v2] Add FDB support

2014-12-15 Thread Tom Gundersen
Hi Alin,

Thanks for the patch!

Overall it looks excellent, my only concern is with the clearing of
FDB entries. Is there any reason to treat this differently than how we
currently treat routes and addresses?

The current logic is that we don't support run-time reconfiguration
(we only apply our (static) config once when the interfaces appear,
and that's it). If there is some config there already we assume that
it is intentional and leave it alone (i.e., if someone else have set
addresses or routes we don't remove them, we just add new ones). In
the future we plan to get a dbus API where networkd can be
reconfigured at run-time (i.e., change which .network file is applied
to a link), and then it definitely would make sense to flush routes
and addresses when removing the .network from the link, but currently
we don't do that at all.

If people want to reconfigure stuff now, they can of course just
restart networkd, but then they should also manually flush the
settings from the kernel, and this (changing the config, restarting
the daemon and flushing) is more of a hack for testing than something
we support.

So the question I have is: can we treat FDB entries the same as routes
and addresses and simply leave them alone rather than flushing them,
and then only add the flushing logic once we start supporting
reconfiguration? If not, can you explain a bit more what the typical
usecase is where flushing is desirable?

As to the naming, I like the suggestion made above to call it VLANId
rather than VLAN.

Cheers,

Tom

On Mon, Dec 15, 2014 at 11:20 AM, Alin Rauta alin.ra...@intel.com wrote:
 Hi,

 Based on your feedback, I've created a new patch for adding the FDB support.
 networkd takes ownership of the FDB table.

 Configuration example:

 cat /etc/systemd/network/em1.network

 [Match]
 Name=em1

 [Network]
 DHCP=v4

 [BridgeFDB]
 MACAddress=04:44:12:34:56:70
 VLAN=2

 [BridgeFDB]
 MACAddress=04:44:12:34:56:69
 VLAN=3

 Let me know what you think.

 Thanks,
 Alin

 Alin Rauta (1):
   Add FDB support

  Makefile.am  |   1 +
  man/systemd.network.xml  |  22 ++
  src/libsystemd/sd-rtnl/rtnl-message.c|  56 -
  src/libsystemd/sd-rtnl/rtnl-types.c  |  15 +-
  src/network/networkd-fdb.c   | 357 
 +++
  src/network/networkd-link.c  |  25 +++
  src/network/networkd-network-gperf.gperf |   2 +
  src/network/networkd-network.c   |  13 +-
  src/network/networkd.h   |  30 +++
  src/systemd/sd-rtnl.h|   4 +
  10 files changed, 514 insertions(+), 11 deletions(-)
  create mode 100644 src/network/networkd-fdb.c

 --
 1.9.3

 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel