[RFC Draft PATCH net-next 0/1] Bridge doc update

[RFC Draft PATCH net-next 0/1] Bridge doc update

[Hangbin Liu]

Hi,

After a long busy period. I got time to check how to update the bridge doc.
Here is the previous discussion we made[1].

In this update. I plan to convert all the bridge description/comments to
the kernel headers. And add sphinx identifiers in the doc to show them
directly. At the same time, I wrote a script to convert the description
in kernel header file to iproute2 man doc. With this, there is no need
to maintain the doc in 2 places.

For the script. I use python docutils to read the rst comments. When dump
the man page. I do it manually to match the current ip link man page style.
I tried rst2man, but the generated man doc will break the current style.
If you have any other better way, please tell me.

[1] https://lore.kernel.org/netdev/5ddac447-c268-e559-a8dc-08ae3d124352@blackwall.org/


Hangbin Liu (1):
  Doc: update bridge doc

 Documentation/networking/bridge.rst |  85 ++++++++++--
 include/uapi/linux/if_bridge.h      |  24 ++++
 include/uapi/linux/if_link.h        | 194 ++++++++++++++++++++++++++++
 3 files changed, 293 insertions(+), 10 deletions(-)

-- 
2.41.0

[RFC Draft PATCH net-next] Doc: update bridge doc

[Hangbin Liu]

This is an example of bridge doc update. In this example I use the
sphinx identifier to insert the structure description in the doc.

I plan to copy all the iproute2 bridge related man docs first.
Please tell me if other doc I need to add.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 Documentation/networking/bridge.rst |  85 ++++++++++--
 include/uapi/linux/if_bridge.h      |  24 ++++
 include/uapi/linux/if_link.h        | 194 ++++++++++++++++++++++++++++
 3 files changed, 293 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/bridge.rst b/Documentation/networking/bridge.rst
index c859f3c1636e..7a877c304478 100644
--- a/Documentation/networking/bridge.rst
+++ b/Documentation/networking/bridge.rst
@@ -4,18 +4,83 @@
 Ethernet Bridging
 =================
 
-In order to use the Ethernet bridging functionality, you'll need the
-userspace tools.
+Introduction
+============
 
-Documentation for Linux bridging is on:
-   https://wiki.linuxfoundation.org/networking/bridge
+A bridge is a way to connect two Ethernet segments together in a protocol
+independent way. Packets are forwarded based on Ethernet address, rather
+than IP address (like a router). Since forwarding is done at Layer 2, all
+protocols can go transparently through a bridge.
 
-The bridge-utilities are maintained at:
-   git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/bridge-utils.git
+Bridge internals
+================
 
-Additionally, the iproute2 utilities can be used to configure
-bridge devices.
+.. kernel-doc:: include/uapi/linux/if_bridge.h
+   :identifiers: __bridge_info
 
-If you still have questions, don't hesitate to post to the mailing list 
-(more info https://lists.linux-foundation.org/mailman/listinfo/bridge).
+.. kernel-doc:: include/uapi/linux/if_bridge.h
+   :identifiers: __port_info
+
+Bridge uAPI
+===========
+
+Bridge netlink attributes
+-------------------------
+
+.. kernel-doc:: include/uapi/linux/if_link.h
+   :doc: The bridge emum defination
+
+Bridge sysfs
+------------
+
+Most of them are same with netlink attributes. What about the read only
+parameters like gc_timer, tcn_timer? Should we doc them?
+
+STP
+===
+
+Multicast
+=========
+
+VLAN
+====
+
+Switchdev
+=========
+
+Netfilter
+=========
+
+FAQ
+===
+
+What does a bridge do?
+----------------------
+
+A bridge transparently relays traffic between multiple network interfaces.
+In plain English this means that a bridge connects two or more physical
+Ethernets together to form one bigger (logical) Ethernet.
+
+Is it protocol independent?
+---------------------------
+
+Yes. The bridge knows nothing about protocols, it only sees Ethernet frames.
+As such, the bridging functionality is protocol independent, and there should
+be no trouble relaying IPX, NetBEUI, IP, IPv6, etc.
+
+Contact Info
+============
+
+The code is currently maintained by Roopa Prabhu <roopa@nvidia.com> and
+Nikolay Aleksandrov <razor@blackwall.org>. Bridge bugs and enhancements
+are discussed on the linux-netdev mailing list netdev@vger.kernel.org and
+bridge@lists.linux-foundation.org.
+
+The list is open to anyone interested: http://vger.kernel.org/vger-lists.html#netdev
+
+External Links
+==============
+
+The old Documentation for Linux bridging is on:
+https://wiki.linuxfoundation.org/networking/bridge
 
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index f95326fce6bb..7e8ee14afc3a 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -52,6 +52,19 @@
 #define BR_STATE_FORWARDING 3
 #define BR_STATE_BLOCKING 4
 
+/**
+ * struct __bridge_info - the bridge information
+ *
+ * @designated_root: designated root
+ *
+ * @bridge_id: bridge id
+ *
+ * @root_path_cost: root path cost
+ *
+ * @max_age: max age
+ *
+ * @hello_time: hello time
+ */
 struct __bridge_info {
 	__u64 designated_root;
 	__u64 bridge_id;
@@ -74,6 +87,17 @@ struct __bridge_info {
 	__u32 gc_timer_value;
 };
 
+/**
+ * struct __port_info - the bridge port information
+ *
+ * @designated_root: designated root
+ *
+ * @designated_bridge: designated bridge
+ *
+ * @port_id: port id
+ *
+ * @designated_port: designated port
+ */
 struct __port_info {
 	__u64 designated_root;
 	__u64 designated_bridge;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index ce3117df9cec..277855ccad1e 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -461,6 +461,200 @@ enum in6_addr_gen_mode {
 
 /* Bridge section */
 
+/**
+ * DOC: The bridge emum defination
+ *
+ * @IFLA_BR_FORWARD_DELAY:
+ *   set the forwarding delay in seconds, ie the time spent in LISTENING
+ *   state (before moving to LEARNING) and in LEARNING state (before moving
+ *   to FORWARDING). Only relevant if STP is enabled. Valid values are
+ *   between 2 and 30.
+ *
+ * @IFLA_BR_HELLO_TIME:
+ *   set the time in seconds between hello packets sent by the bridge,
+ *   when it is a root bridge or a designated bridges. Only relevant if
+ *   STP is enabled. Valid values are between 1 and 10.
+ *
+ * @IFLA_BR_MAX_AGE:
+ *   set the hello packet timeout, ie the time in seconds until another
+ *   bridge in the spanning tree is assumed to be dead, after reception of
+ *   its last hello message. Only relevant if STP is enabled. Valid values
+ *   are between 6 and 40.
+ *
+ * @IFLA_BR_AGEING_TIME:
+ *   configure the bridge's FDB entries ageing time, ie the number of
+ *   seconds a MAC address will be kept in the FDB after a packet has been
+ *   received from that address. after this time has passed, entries are
+ *   cleaned up.
+ *
+ * @IFLA_BR_STP_STATE:
+ *   turn spanning tree protocol on (*IFLA_BR_STP_STATE* > 0) or off
+ *   (*IFLA_BR_STP_STATE* == 0). for this bridge.
+ *
+ * @IFLA_BR_PRIORITY:
+ *   set this bridge's spanning tree priority, used during STP root bridge
+ *   election.  *IFLA_BR_PRIORITY* is a 16bit unsigned integer.
+ *
+ * @IFLA_BR_VLAN_FILTERING:
+ *   turn VLAN filtering on (*IFLA_BR_VLAN_FILTERING* > 0) or off
+ *   (*IFLA_BR_VLAN_FILTERING* == 0). When disabled, the bridge will not
+ *   consider the VLAN tag when handling packets.
+ *
+ * @IFLA_BR_VLAN_PROTOCOL:
+ *   set the protocol used for VLAN filtering.
+ *
+ * @IFLA_BR_GROUP_FWD_MASK:
+ *   set the group forward mask. This is the bitmask that is applied to
+ *   decide whether to forward incoming frames destined to link-local
+ *   addresses, ie addresses of the form 01:80:C2:00:00:0X (defaults to 0,
+ *   ie the bridge does not forward any linklocal frames coming on this port).
+ *
+ * @IFLA_BR_ROOT_ID:
+ *   Bridge root id.
+ *
+ * @IFLA_BR_BRIDGE_ID:
+ *   Bridge id.
+ *
+ * @IFLA_BR_ROOT_PORT:
+ *   Bridge root port.
+ *
+ * @IFLA_BR_ROOT_PATH_COST:
+ *   Bridge path cost.
+ *
+ * @IFLA_BR_TOPOLOGY_CHANGE:
+ *   Bridge topology change.
+ *
+ * @IFLA_BR_TOPOLOGY_CHANGE_DETECTED:
+ *   Bridge topology change detected.
+ *
+ * @IFLA_BR_HELLO_TIMER:
+ *   Bridge hello timer.
+ *
+ * @IFLA_BR_TCN_TIMER:
+ *   Bridge TCN timer.
+*
+ * @IFLA_BR_TOPOLOGY_CHANGE_TIMER:
+ *   Bridge topology change timer.
+ *
+ * @IFLA_BR_GC_TIMER:
+ *   Bridge gc timer.
+ *
+ * @IFLA_BR_GROUP_ADDR:
+ *   set the MAC address of the multicast group this bridge uses for STP.
+ *   The address must be a link-local address in standard Ethernet MAC address
+ *   format, ie an address of the form 01:80:C2:00:00:0X, with X in [0, 4..f].
+ *
+ * @IFLA_BR_FDB_FLUSH:
+ *   Bridge FDB flush.
+ *
+ * @IFLA_BR_MCAST_ROUTER:
+ *   set bridge's multicast router if IGMP snooping is enabled.
+ *   *IFLA_BR_MCAST_ROUTER* is an integer value having the following meaning:
+ *
+ *     * **0** - disabled.
+ *     * **1** - automatic (queried).
+ *     * **2** - permanently enabled.
+ *
+ * @IFLA_BR_MCAST_SNOOPING:
+ *   turn multicast snooping on (*IFLA_BR_MCAST_SNOOPING* > 0) or off
+ *   (*IFLA_BR_MCAST_SNOOPING* == 0).
+ *
+ * @IFLA_BR_MCAST_QUERY_USE_IFADDR:
+ *   whether to use the bridge's own IP address as source address for IGMP
+ *   queries (*IFLA_BR_MCAST_QUERY_USE_IFADDR* > 0) or the default of 0.0.0.0
+ *   (*IFLA_BR_MCAST_QUERY_USE_IFADDR* == 0).
+ *
+ * @IFLA_BR_MCAST_QUERIER:
+ *   enable (*IFLA_BR_MULTICAST_QUERIER* > 0) or disable
+ *   (*IFLA_BR_MULTICAST_QUERIER* == 0) IGMP querier, ie sending of multicast
+ *   queries by the bridge (default: disabled).
+ *
+ * @IFLA_BR_MCAST_HASH_ELASTICITY:
+ *   set multicast database hash elasticity, ie the maximum chain length in
+ *   the multicast hash table (defaults to 4).
+ *
+ * @IFLA_BR_MCAST_HASH_MAX:
+ *   set maximum size of multicast hash table (defaults to 512, value must
+ *   be a power of 2).
+ *
+ * @IFLA_BR_MCAST_LAST_MEMBER_CNT:
+ *   set multicast last member count, ie the number of queries the bridge
+ *   will send before stopping forwarding a multicast group after a "leave"
+ *   message has been received (defaults to 2).
+ *
+ * @IFLA_BR_MCAST_STARTUP_QUERY_CNT:
+ *   set the number of IGMP queries to send during startup phase (defaults
+ *   to 2).
+ *
+ * @IFLA_BR_MCAST_LAST_MEMBER_INTVL:
+ *   interval between queries to find remaining members of a group, after
+ *   a "leave" message is received.
+ *
+ * @IFLA_BR_MCAST_MEMBERSHIP_INTVL:
+ *   delay after which the bridge will leave a group, if no membership
+ *   reports for this group are received.
+ *
+ * @IFLA_BR_MCAST_QUERIER_INTVL:
+ *   interval between queries sent by other routers. if no queries are seen
+ *   after this delay has passed, the bridge will start to send its own
+ *   queries (as if **IFLA_BR_MCAST_QUERIER_INTVL** was enabled).
+ *
+ * @IFLA_BR_MCAST_QUERY_INTVL:
+ *   interval between queries sent by the bridge after the end of the
+ *   startup phase.
+ *
+ * @IFLA_BR_MCAST_QUERY_RESPONSE_INTVL:
+ *   set the Max Response Time/Maximum Response Delay for IGMP/MLD queries
+ *   sent by the bridge.
+ *
+ * @IFLA_BR_MCAST_STARTUP_QUERY_INTVL:
+ *   interval between queries in the startup phase.
+ *
+ * @IFLA_BR_NF_CALL_IPTABLES:
+ *   enable (*NF_CALL_IPTABLES* > 0) or disable (*NF_CALL_IPTABLES* == 0)
+ *   iptables hooks on the bridge.
+ *
+ * @IFLA_BR_NF_CALL_IP6TABLES:
+ *   enable (*NF_CALL_IP6TABLES* > 0) or disable (*NF_CALL_IP6TABLES* == 0)
+ *   ip6tables hooks on the bridge.
+ *
+ * @IFLA_BR_NF_CALL_ARPTABLES:
+ *   enable (*NF_CALL_ARPTABLES* > 0) or disable (*NF_CALL_ARPTABLES* == 0)
+ *   arptables hooks on the bridge.
+ *
+ * @IFLA_BR_VLAN_DEFAULT_PVID:
+ *   set the default PVID (native/untagged VLAN ID) for this bridge.
+ *
+ * @IFLA_BR_PAD:
+ *   Bridge attr PAD
+ *
+ * @IFLA_BR_VLAN_STATS_ENABLED:
+ *   enable (*IFLA_BR_VLAN_STATS_ENABLED* == 1) or disable
+ *   (*IFLA_BR_VLAN_STATS_ENABLED* == 0) per-VLAN stats accounting.
+ *
+ * @IFLA_BR_MCAST_STATS_ENABLED:
+ *   enable (*IFLA_BR_MCAST_STATS_ENABLED* > 0) or disable
+ *   (*IFLA_BR_MCAST_STATS_ENABLED* == 0) multicast (IGMP/MLD) stats
+ *   accounting.
+ *
+ * @IFLA_BR_MCAST_IGMP_VERSION:
+ *   set the IGMP version.
+ *
+ * @IFLA_BR_MCAST_MLD_VERSION:
+ *   set the MLD version.
+ *
+ * @IFLA_BR_VLAN_STATS_PER_PORT:
+ *   enable (*IFLA_BR_VLAN_STATS_PER_PORT* == 1) or disable
+ *   (*IFLA_BR_VLAN_STATS_PER_PORT* == 0) per-VLAN per-port stats accounting.
+ *   Can be changed only when there are no port VLANs configured.
+ *
+ * @IFLA_BR_MULTI_BOOLOPT:
+ *   Bridge multi bool options.
+ *
+ * @IFLA_BR_MCAST_QUERIER_STATE:
+ *   Bridge mcast querier state.
+ */
+
 enum {
 	IFLA_BR_UNSPEC,
 	IFLA_BR_FORWARD_DELAY,
-- 
2.41.0

[RFC Draft PATCH iproute2-next] tools: add a tool to generate bridge man doc

[Hangbin Liu]

This tool will generate the ip link bridge parameters and find related
attr description in if_link.h. And convert to the new man doc.

To use it. You need to run the script first before your patch. With this
we can update the man page to latest version. e.g.

./generate_bridge_man.py -i iproute2_dir -n net-next_dir

The script will generate a ip-link.8.out page. You can check if it's OK
to move to ip-link.8.in.

After adding new parameter. You need to re-run the script to generate the man
doc from net-next header file.

The script check the existing parameter. So if the attr in net-next has not
added to iproute2, it will not generate the man doc.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 man/man8/ip-link.8.in        |   2 +
 tools/generate_bridge_man.py | 188 +++++++++++++++++++++++++++++++++++
 2 files changed, 190 insertions(+)
 create mode 100755 tools/generate_bridge_man.py

diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 8f07de9a8a25..cbf6d9a9d9c4 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -1611,6 +1611,7 @@ For a link of type
 the following additional arguments are supported:
 
 .BI "ip link add " DEVICE " type bridge "
+.\" bridge man doc starts
 [
 .BI ageing_time " AGEING_TIME "
 ] [
@@ -1887,6 +1888,7 @@ arptables hooks on the bridge.
 
 
 .in -8
+.\" bridge man doc ends
 
 .TP
 MACsec Type Support
diff --git a/tools/generate_bridge_man.py b/tools/generate_bridge_man.py
new file mode 100755
index 000000000000..45947abda382
--- /dev/null
+++ b/tools/generate_bridge_man.py
@@ -0,0 +1,188 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-only
+
+import re, sys, os
+import argparse
+from docutils import core
+
+man_bridge = ""
+
+def handle_args():
+    parser = argparse.ArgumentParser(description="""Convert bridge header
+                                     comments to iproute2 man doc.""")
+    parser.add_argument('-i', '--iproute2-dir', required=True,
+                        help='iproute code path')
+    parser.add_argument('-n', '--net-dir', required=True,
+                        help='net-next code path')
+    args = parser.parse_args()
+    return args
+
+def write_man_doc(input_file, output_file):
+    try:
+        with open(input_file, 'r') as infile, open(output_file, 'w') as outfile:
+            remove_flag = False
+            for line in infile:
+                if line.find("bridge man doc starts") != -1:
+                    remove_flag = True
+                    outfile.write(line)
+                    outfile.write(man_bridge + '\n')
+                elif remove_flag and line.find("bridge man doc ends") != -1:
+                    remove_flag = False
+                elif not remove_flag:
+                    outfile.write(line)
+    except Exception as e:
+        sys.exit(e)
+
+def append_man_line(line, end = '\n'):
+    global man_bridge
+    man_bridge = man_bridge + line + end
+
+def rst2man(node):
+    if not hasattr(node, 'astext'):
+        return
+
+    if node.tagname == 'bullet_list':
+        append_man_line('.in +8\n.sp')
+    if node.tagname == 'list_item':
+        append_man_line('')
+    elif node.tagname == 'emphasis':
+        append_man_line('.I ', end = '')
+    elif node.tagname == 'strong':
+        append_man_line('.B ', end = '')
+
+    for child_node in node.children:
+        rst2man(child_node)
+
+    if len(node.children) == 0:
+        text = node.astext().strip()
+        append_man_line(text)
+
+    if node.tagname == 'bullet_list':
+        append_man_line('.in -8')
+
+def convert2man(params):
+    first = True
+    for p in params:
+        if first:
+            first = False
+            append_man_line('[')
+        else:
+            append_man_line(' [')
+        append_man_line('.BI ' + p + ' " ' + params[p]['param'] + ' "')
+        append_man_line(']', end = '')
+
+    append_man_line('\n\n.in +8\n.sp')
+
+    for p in params:
+        if 'alias' in params[p]:
+            # TODO: fix the vlan_protocol output, e.g. {|}
+            append_man_line('\n.BR ' + p + ' " ' + params[p]['alias'] + ' "')
+        else:
+            append_man_line('\n.BI ' + p + ' " ' + params[p]['param'] + ' "')
+        # Add the '- ' at begining of each description
+        append_man_line('- ', end = '')
+        if 'doc' in params[p]:
+            rst_nodes = core.publish_doctree(params[p]['doc'])
+            rst2man(rst_nodes)
+
+def strip_doc_line(line, param):
+    # Remove the first " *   "
+    line = line[5:-1]
+    # replace attr with param, e.g. IFLA_BR_STP_STATE -> STP_STATE
+    line = line.replace(param['attr'], param['param'])
+    return line
+
+def get_attr(line):
+    match = re.search(r'@(\w+):', line)
+    if match:
+        return match.group(1)
+    return None
+
+# Deal some irregular namings
+def param2attr(param):
+    param = param.replace('count', 'cnt')
+    param = param.replace('interval', 'intvl')
+    param = param.replace('group_address', 'group_addr')
+    return param.upper()
+
+def get_bridge_doc(bridge_header, params):
+    find_attr = False
+    p = None
+    try:
+        with open(bridge_header, 'r') as f:
+            line = f.readline()
+            # Find the start of the doc
+            while line.find("DOC: The bridge emum defination") == -1:
+                if not line:
+                    print("Unable to find bridge DOC");
+                    return None
+                line = f.readline()
+            # Till the end of the doc
+            while line.find("*/") == -1:
+                line = f.readline()
+                # Start of a parameter
+                if line.find(' * @') == 0:
+                    find_attr = False
+                    for p in params:
+                        if line.find(param2attr(p) + ':') != -1:
+                            attr = get_attr(line)
+                            if attr is not None:
+                                params[p]['attr'] = attr
+                                find_attr = True
+                            break
+                elif find_attr and p:
+                    if 'doc' in params[p]:
+                        params[p]['doc'] = params[p]['doc'] + '\n' + strip_doc_line(line, params[p])
+                    else:
+                        params[p]['doc'] = strip_doc_line(line, params[p])
+
+    except Exception as e:
+        sys.exit(e)
+
+# replace multi text
+def strip_line(line):
+    return line.replace(' ', '').replace('"', '').replace('\\n', '').strip()
+
+def get_bridge_parameter(bridge_file):
+    params = {}
+    try:
+        with open(bridge_file, 'r') as f:
+            line = f.readline()
+            # Find the start of parameters
+            while line.find("Usage: ... bridge") == -1:
+                line = f.readline()
+            # Till the end of the parameters
+            while line.find("Where:") == -1:
+                line = f.readline()
+                parts = line.strip().split()
+                if len(parts) >= 4:
+                    if parts[1] == '[':
+                        params[parts[2]] = {}
+                        params[parts[2]]['param'] = parts[3]
+            # Till the end of usage function to get the alias
+            while line.find(");") == -1:
+                parts = line.strip().replace('Where:', '').split(':=')
+                alias = strip_line(parts[0])
+                for p in params:
+                    if params[p]['param'] == alias:
+                        params[p]['alias'] = strip_line(parts[1])
+                line = f.readline()
+    except Exception as e:
+        sys.exit(e)
+
+    return params
+
+def main():
+    args = handle_args()
+    bridge_file = args.iproute2_dir.rstrip('/') + "/ip/iplink_bridge.c"
+    ip_link_in = args.iproute2_dir.rstrip('/') + "/man/man8/ip-link.8.in"
+    ip_link_out = args.iproute2_dir.rstrip('/') + "/man/man8/ip-link.8.out"
+    bridge_header = args.net_dir.rstrip('/') + '/include/uapi/linux/if_link.h'
+
+    bridge_params = get_bridge_parameter(bridge_file)
+    get_bridge_doc(bridge_header, bridge_params)
+    convert2man(bridge_params)
+    write_man_doc(ip_link_in, ip_link_out)
+
+if __name__ == '__main__':
+    main()
-- 
2.41.0

Re: [RFC Draft PATCH net-next 0/1] Bridge doc update

[Stephen Hemminger]

On Wed, 13 Sep 2023 17:28:52 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:

> Hi,
> 
> After a long busy period. I got time to check how to update the
> bridge doc. Here is the previous discussion we made[1].
> 
> In this update. I plan to convert all the bridge description/comments
> to the kernel headers. And add sphinx identifiers in the doc to show
> them directly. At the same time, I wrote a script to convert the
> description in kernel header file to iproute2 man doc. With this,
> there is no need to maintain the doc in 2 places.
> 
> For the script. I use python docutils to read the rst comments. When
> dump the man page. I do it manually to match the current ip link man
> page style. I tried rst2man, but the generated man doc will break the
> current style. If you have any other better way, please tell me.
> 
> [1]
> https://lore.kernel.org/netdev/5ddac447-c268-e559-a8dc-08ae3d124352@blackwall.org/
> 
> 
> Hangbin Liu (1):
>   Doc: update bridge doc
> 
>  Documentation/networking/bridge.rst |  85 ++++++++++--
>  include/uapi/linux/if_bridge.h      |  24 ++++
>  include/uapi/linux/if_link.h        | 194
> ++++++++++++++++++++++++++++ 3 files changed, 293 insertions(+), 10
> deletions(-)
> 

Not sure this is good idea.
- you are special casing bridge documentation and there is lots of
  other parts of iproute2
- you are introducing a dependency on python in iproute2
- the kernel headers in iproute2 come from sanitized kernel headers. So
  fixing the documentation would take longer.

What problem is this trying to solve?

Re: [RFC Draft PATCH net-next 0/1] Bridge doc update

[Stephen Hemminger]

On Wed, 13 Sep 2023 17:28:52 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:

> Hi,
> 
> After a long busy period. I got time to check how to update the
> bridge doc. Here is the previous discussion we made[1].
> 
> In this update. I plan to convert all the bridge description/comments
> to the kernel headers. And add sphinx identifiers in the doc to show
> them directly. At the same time, I wrote a script to convert the
> description in kernel header file to iproute2 man doc. With this,
> there is no need to maintain the doc in 2 places.
> 
> For the script. I use python docutils to read the rst comments. When
> dump the man page. I do it manually to match the current ip link man
> page style. I tried rst2man, but the generated man doc will break the
> current style. If you have any other better way, please tell me.
> 
> [1]
> https://lore.kernel.org/netdev/5ddac447-c268-e559-a8dc-08ae3d124352@blackwall.org/
> 
> 
> Hangbin Liu (1):
>   Doc: update bridge doc
> 
>  Documentation/networking/bridge.rst |  85 ++++++++++--
>  include/uapi/linux/if_bridge.h      |  24 ++++
>  include/uapi/linux/if_link.h        | 194
> ++++++++++++++++++++++++++++ 3 files changed, 293 insertions(+), 10
> deletions(-)
> 

Not sure this is good idea.
- you are special casing bridge documentation and there is lots of
  other parts of iproute2
- you are introducing a dependency on python in iproute2
- the kernel headers in iproute2 come from sanitized kernel headers. So
  fixing the documentation would take longer.

What problem is this trying to solve

Re: [RFC Draft PATCH net-next 0/1] Bridge doc update

[Hangbin Liu]

On Wed, Sep 13, 2023 at 04:22:24AM -0700, Stephen Hemminger wrote:
> On Wed, 13 Sep 2023 17:28:52 +0800
> Hangbin Liu <liuhangbin@gmail.com> wrote:
> 
> > Hi,
> > 
> > After a long busy period. I got time to check how to update the
> > bridge doc. Here is the previous discussion we made[1].
> > 
> > In this update. I plan to convert all the bridge description/comments
> > to the kernel headers. And add sphinx identifiers in the doc to show
> > them directly. At the same time, I wrote a script to convert the
> > description in kernel header file to iproute2 man doc. With this,
> > there is no need to maintain the doc in 2 places.
> > 
> > For the script. I use python docutils to read the rst comments. When
> > dump the man page. I do it manually to match the current ip link man
> > page style. I tried rst2man, but the generated man doc will break the
> > current style. If you have any other better way, please tell me.
> > 
> > [1]
> > https://lore.kernel.org/netdev/5ddac447-c268-e559-a8dc-08ae3d124352@blackwall.org/
> > 
> > 
> > Hangbin Liu (1):
> >   Doc: update bridge doc
> > 
> >  Documentation/networking/bridge.rst |  85 ++++++++++--
> >  include/uapi/linux/if_bridge.h      |  24 ++++
> >  include/uapi/linux/if_link.h        | 194
> > ++++++++++++++++++++++++++++ 3 files changed, 293 insertions(+), 10
> > deletions(-)
> > 
> 
> Not sure this is good idea.
> - you are special casing bridge documentation and there is lots of
>   other parts of iproute2

   Yes, the patch's purpose is to save the work for bridge doc(at present).
   So only the bridge part of iproute2 man page will be updated. I added a
   tag at the start/end of the bridge part. Other parts will not be touched.

> - you are introducing a dependency on python in iproute2

    There is no need to build it. I put it in the tools folder. Any one can
    choose to use or not use it. So I don't think it count for dependency.

> - the kernel headers in iproute2 come from sanitized kernel headers. So
>   fixing the documentation would take longer.

    It doesn't matter. The python script only grab the attr description from
    net-next and convert it to man doc. It doesn't care about the kernel
    header in iproute2.
> 
> What problem is this trying to solve?

The bridge doc in kernel[1] is too old. There are a lot of details hide in
the kernel code and new developer is hard to catch up. So the kernel bridge
doc page need an update. 

You could say the iproute2 bridge documentation is new and updated. We can
take it as the new bridge doc. But iprotue2 bridge doc only contains uAPI.
The kernel API doc is still needed.

On the other hand, the bridge developers already speed a lot effort on iproute2
bridge documentation. It would introduce more work and complex to maintain
the kernel and bridge doc at the same time.

So I suggest to only maintain the kernel doc. And convert the kernel part
directly to iproute2 man page. With this, we can maintain the doc in one
place and update them both on kernel and iproute2.

[1] https://www.kernel.org/doc/Documentation/networking/bridge.rst

Thanks
Hangbin

Re: [RFC Draft PATCH net-next 0/1] Bridge doc update

[Hangbin Liu]

Hi Nikolay, Roopa,

Do you have any comments for this RFC?

Thanks
Hangbin
On Wed, Sep 13, 2023 at 05:28:52PM +0800, Hangbin Liu wrote:
> Hi,
> 
> After a long busy period. I got time to check how to update the bridge doc.
> Here is the previous discussion we made[1].
> 
> In this update. I plan to convert all the bridge description/comments to
> the kernel headers. And add sphinx identifiers in the doc to show them
> directly. At the same time, I wrote a script to convert the description
> in kernel header file to iproute2 man doc. With this, there is no need
> to maintain the doc in 2 places.
> 
> For the script. I use python docutils to read the rst comments. When dump
> the man page. I do it manually to match the current ip link man page style.
> I tried rst2man, but the generated man doc will break the current style.
> If you have any other better way, please tell me.
> 
> [1] https://lore.kernel.org/netdev/5ddac447-c268-e559-a8dc-08ae3d124352@blackwall.org/
> 
> 
> Hangbin Liu (1):
>   Doc: update bridge doc
> 
>  Documentation/networking/bridge.rst |  85 ++++++++++--
>  include/uapi/linux/if_bridge.h      |  24 ++++
>  include/uapi/linux/if_link.h        | 194 ++++++++++++++++++++++++++++
>  3 files changed, 293 insertions(+), 10 deletions(-)
> 
> -- 
> 2.41.0
> 

Re: [RFC Draft PATCH net-next 0/1] Bridge doc update

[Nikolay Aleksandrov]

On 9/20/23 12:19, Hangbin Liu wrote:
> Hi Nikolay, Roopa,
> 
> Do you have any comments for this RFC?
> 
> Thanks
> Hangbin
> On Wed, Sep 13, 2023 at 05:28:52PM +0800, Hangbin Liu wrote:
>> Hi,
>>
>> After a long busy period. I got time to check how to update the bridge doc.
>> Here is the previous discussion we made[1].
>>
>> In this update. I plan to convert all the bridge description/comments to
>> the kernel headers. And add sphinx identifiers in the doc to show them
>> directly. At the same time, I wrote a script to convert the description
>> in kernel header file to iproute2 man doc. With this, there is no need
>> to maintain the doc in 2 places.
>>
>> For the script. I use python docutils to read the rst comments. When dump
>> the man page. I do it manually to match the current ip link man page style.
>> I tried rst2man, but the generated man doc will break the current style.
>> If you have any other better way, please tell me.
>>
>> [1] https://lore.kernel.org/netdev/5ddac447-c268-e559-a8dc-08ae3d124352@blackwall.org/
>>
>>
>> Hangbin Liu (1):
>>    Doc: update bridge doc
>>
>>   Documentation/networking/bridge.rst |  85 ++++++++++--
>>   include/uapi/linux/if_bridge.h      |  24 ++++
>>   include/uapi/linux/if_link.h        | 194 ++++++++++++++++++++++++++++
>>   3 files changed, 293 insertions(+), 10 deletions(-)
>>
>> -- 
>> 2.41.0
>>

Hi Hangbin,
I support all efforts to improve documentation, but I do share the same 
concerns that Stephen has already voiced. I don't think we should be 
generating the man page from the kernel docs, IMO it would be simpler
and easier for everyone to support both docs - one is for the user-space
iproute2 commands, the other could go into the kernel api details. All
attribute descriptions can still be added to headers, that would be very
valuable on its own. I prefer to have the freedom to change the docs 
format in any way, generating them from comments is kind of limiting.
The purpose of each document is different and it will be difficult
to combine them for a man page. It would be much easier for everyone
to add user-related command descriptions and examples in iproute2's 
documentation, and to add kernel-specific (or uapi) documentation to the
kernel doc. We can add references for each with a short description.
W.r.t the kernel doc topics covered, I think the list is a good start.

Thank you,
  Nik

Re: [RFC Draft PATCH net-next 0/1] Bridge doc update

[Hangbin Liu]

On Wed, Sep 20, 2023 at 01:38:44PM +0300, Nikolay Aleksandrov wrote:
> > On Wed, Sep 13, 2023 at 05:28:52PM +0800, Hangbin Liu wrote:
> > > Hi,
> > > 
> > > After a long busy period. I got time to check how to update the bridge doc.
> > > Here is the previous discussion we made[1].
> > > 
> > > In this update. I plan to convert all the bridge description/comments to
> > > the kernel headers. And add sphinx identifiers in the doc to show them
> > > directly. At the same time, I wrote a script to convert the description
> > > in kernel header file to iproute2 man doc. With this, there is no need
> > > to maintain the doc in 2 places.
> > > 
> > > For the script. I use python docutils to read the rst comments. When dump
> > > the man page. I do it manually to match the current ip link man page style.
> > > I tried rst2man, but the generated man doc will break the current style.
> > > If you have any other better way, please tell me.
> > > 
> > > [1] https://lore.kernel.org/netdev/5ddac447-c268-e559-a8dc-08ae3d124352@blackwall.org/
> > > 
> Hi Hangbin,
> I support all efforts to improve documentation, but I do share the same
> concerns that Stephen has already voiced. I don't think we should be
> generating the man page from the kernel docs, IMO it would be simpler
> and easier for everyone to support both docs - one is for the user-space
> iproute2 commands, the other could go into the kernel api details. All
> attribute descriptions can still be added to headers, that would be very
> valuable on its own. I prefer to have the freedom to change the docs format
> in any way, generating them from comments is kind of limiting.
> The purpose of each document is different and it will be difficult
> to combine them for a man page. It would be much easier for everyone
> to add user-related command descriptions and examples in iproute2's
> documentation, and to add kernel-specific (or uapi) documentation to the
> kernel doc. We can add references for each with a short description.

Hi Nikolay,

Thanks for the feedback. I agree that it's more reasonable to have
different docs for user-space and kernel api. As long as our bridge developers
satisfied to maintain these 2 docs at the same time, I'm totally OK to drop
this bloated convert tool.

> W.r.t the kernel doc topics covered, I think the list is a good start.

Thanks, I will add more parts and re-post it next month.

Regards
Hangbin