T4659: op-mode: Display bridge interface details

[jack9603301]

Change Summary

Display bridge interface details

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Code style update (formatting, renaming)
• [ ] Refactoring (no functional changes)
• [ ] Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• [ ] Other (please describe):

Related Task(s)

• https://phabricator.vyos.net/T4659

Component(s) name

op-mode

Proposed changes

Display bridge interface details

How to test

Checklist:

• [x] I have read the CONTRIBUTING document
• [x] I have linked this PR to one or more Phabricator Task(s)
• [ ] I have run the components SMOKETESTS if applicable
• [x] My commit headlines contain a valid Task id
• [ ] My change requires a change to the documentation
• [ ] I have updated the documentation accordingly

[sever-sever]

root@r14:/home/vyos# ./bridge.py 
  File "/home/vyos/./bridge.py", line 208
    else if detail:
         ^
SyntaxError: invalid syntax
root@r14:/home/vyos# 

[jack9603301]

root@r14:/home/vyos# ./bridge.py 
  File "/home/vyos/./bridge.py", line 208
    else if detail:
         ^
SyntaxError: invalid syntax
root@r14:/home/vyos# 

done

[sever-sever]

root@r14:/home/vyos# ./bridge.py 
  File "/home/vyos/./bridge.py", line 208
    else if detail:
         ^
SyntaxError: invalid syntax
root@r14:/home/vyos# 

done

Shows nothing:

root@r14:/home/vyos# sudo ip link show type bridge
6: br0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether 52:54:00:c7:31:bc brd ff:ff:ff:ff:ff:ff
root@r14:/home/vyos# 
root@r14:/home/vyos# ./bridge.py show_bridge --detail --interface br0
root@r14:/home/vyos# 
root@r14:/home/vyos# 

[jack9603301]

root@r14:/home/vyos# ./bridge.py 
  File "/home/vyos/./bridge.py", line 208
    else if detail:
         ^
SyntaxError: invalid syntax
root@r14:/home/vyos# 

done

Shows nothing:

root@r14:/home/vyos# sudo ip link show type bridge
6: br0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether 52:54:00:c7:31:bc brd ff:ff:ff:ff:ff:ff
root@r14:/home/vyos# 
root@r14:/home/vyos# ./bridge.py show_bridge --detail --interface br0
root@r14:/home/vyos# 
root@r14:/home/vyos# 

I fixed and tested this problem and I slightly fixed a bug that happened in show_vlan

[jack9603301]

done

The convention for translating op-mode functions 'show_*' to API commands is the Pascal case of function_name + file_name, so in this case, 'show_bridge' would define an API query: 'ShowBridgeBridge' Perhaps you might call the function 'show_detail', hence 'ShowDetailBridge'

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions[bot]]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[jack9603301]

The logic of show_detail needs an adjustment: if you look at python/vyos/opmode.py 'run', the common scenario is to provide a default for 'formatted_output' if 'raw' is False --- this means that in the CLI, one gets formatted output; in the API one gets raw output in the form of a dict. So several changes are suggested: (1) drop the detail argument: the logic should be if raw: data = _get_bridge_json ... etc. else: _get_bridge_details One can then add the nexthop_group arg to the 'else' condition as a nested conditional. (2) Having dropped the detail argument, the nexthop_group arg should be listed as optional, by using the type annotation 'typing.Optional[bool]'; compare version.py for an example.

You may also consider wrapping your _get_bridge_json and the subsequent load in a function called _get_raw_data_detail, to mirror the other analogous functions in bridge.py.

done

[jack9603301]

done

[jestabro]

I realize one error in my previous comment: you will want to handle the nexthop_group option in the raw output also --- the problem with that is that there is no json option for 'vtysh -c "show interface {iface} nexthop-group"', consequently the output to the API will be a single string under the 'result' field. I suggest you make the fixes as suggested in the patch below, and then consider how to format the output in a dict; you can compare the (simpler) case in op_mode/storage.py.

The patch below makes some changes to the function names to make the distinction show/show_detail more clear; it uses the term 'detail' instead of 'details''; it adds the nexthop_group case to the 'raw' case, although it is just the string output as mentioned above.

I think 'detail' instead of 'details' in the CLI is preferable as well.

From 55a3affb0c8f7a670fadcdb7c0c703ad305c89b3 Mon Sep 17 00:00:00 2001
From: John Estabrook <[email protected]>
Date: Mon, 12 Sep 2022 12:04:41 -0500
Subject: [PATCH] fix

---
 src/op_mode/bridge.py | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/op_mode/bridge.py b/src/op_mode/bridge.py
index 4202aafc..ad4f8669 100755
--- a/src/op_mode/bridge.py
+++ b/src/op_mode/bridge.py
@@ -164,17 +164,24 @@ def _get_formatted_output_mdb(data):
     output = tabulate(data_entries, headers)
     return output

-def _get_bridge_details(iface):
+def _get_bridge_detail(iface):
     """Get interface detail statistics"""
     return call(f'vtysh -c "show interface {iface}"')

-def _get_bridge_nexthop_group(iface):
+def _get_bridge_detail_nexthop_group(iface):
     """Get interface detail nexthop_group statistics"""
     return call(f'vtysh -c "show interface {iface} nexthop-group"')

-def _get_bridge_json(iface):
+def _get_bridge_detail_raw(iface):
     """Get interface detail json statistics"""
-    return cmd(f'vtysh -c "show interface {iface} json"')
+    data =  cmd(f'vtysh -c "show interface {iface} json"')
+    data_dict = json.loads(data)
+    return data_dict
+
+def _get_bridge_detail_nexthop_group_raw(iface):
+    """Get interface detail nexthop_group statistics"""
+    out = cmd(f'vtysh -c "show interface {iface} nexthop-group"')
+    return out

 def show(raw: bool):
     bridge_data = _get_raw_data_summary()
@@ -209,14 +216,15 @@ def show_mdb(raw: bool, interface: str):

 def show_detail(raw: bool, nexthop_group: typing.Optional[bool], interface: str):
     if raw:
-        data = _get_bridge_json(interface)
-        data_dict = json.loads(data)
-        return data_dict
+        if nexthop_group:
+            return _get_bridge_detail_nexthop_group_raw(interface)
+        else:
+            return _get_bridge_detail_raw(interface)
     else:
         if nexthop_group:
-            _get_bridge_nexthop_group(interface)
+            return _get_bridge_detail_nexthop_group(interface)
         else:
-            _get_bridge_details(interface)
+            return _get_bridge_detail(interface)


 if __name__ == '__main__':
-- 
2.37.2

[jack9603301]

@jestabro done

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions[bot]]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[c-po]

Please remove those build artifacts from the commit

[jack9603301]

Please remove those build artifacts from the commit

done

I have a question: have these two files been moved to a new place or has XDP support been removed?

currently deleted

[c-po]

The XDP scripts got removed as we will go the VPP way (see blog post)

[jack9603301]

waiting to be merged