Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(pipelined): More mypy fixes #12884

Merged
merged 6 commits into from
Jun 2, 2022
Merged

chore(pipelined): More mypy fixes #12884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1ced778
chore(pipelined): Remove commented out print statements
voisey May 25, 2022
a90af48
chore(pipelined): Fix "Not all arguments converted during string form…
voisey May 25, 2022
eb7a3f4
chore(pipelined): Fix 'Function "builtins.hex" is not valid as a type'
voisey May 25, 2022
90183b4
chore(pipelined): Fix incompatible argument with superclass
voisey May 25, 2022
a96d96e
chore(pipelined): Fix "Incompatible types in assignment"
voisey May 25, 2022
ad5645e
chore(pipelined): Remove more commented out code
voisey Jun 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions lte/gateway/python/magma/pipelined/openflow/flows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,17 +654,17 @@ def _check_resubmit_action(actions, parser):


def send_stats_request(
datapath, tbl_num, cookie: hex = 0,
cookie_mask: hex = 0, retries: int = 3,
datapath, tbl_num, cookie: int = 0,
cookie_mask: int = 0, retries: int = 3,
):
"""
Send a stats request msg
Args:
datapath (ryu.controller.controller.Datapath):
Datapath to query from
table (int): Table number to query for
cookie (hex): cookie value for the request
cookie_mask (hex): cookie mask for the request
cookie (int): a hex cookie value for the request
cookie_mask (int): a hex cookie mask for the request
Comment on lines +666 to +667
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "hex" mean in this context?

Copy link
Contributor Author

@voisey voisey Jun 1, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I meant things like this which is input to the function I've modified here, for example.

Note that Python seems to consider it to be an integer as I have declared here:

>>> OVS_COOKIE_MATCH_ALL = 0xffffffff
>>> type(OVS_COOKIE_MATCH_ALL)
<class 'int'>

though the built-in hex function actually returns strings:

>>> int(str(OVS_COOKIE_MATCH_ALL), 0)
4294967295
>>> hex(4294967295)
'0xffffffff'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All right, so it is just an int for Python. I guess they write it in hexadecimal because the variable is used as a bitmask.

retries (int): retry attempts on failure
"""
ofproto, parser = datapath.ofproto, datapath.ofproto_parser
Expand Down
4 changes: 2 additions & 2 deletions lte/gateway/python/magma/pipelined/qos/qos_tc_impl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@

from lte.protos.policydb_pb2 import FlowMatch

from .tc_ops import TcOpsBase
from .tc_ops_cmd import TcOpsCmd, argSplit, run_cmd
from .tc_ops_pyroute2 import TcOpsPyRoute2
from .types import QosInfo
from .utils import IdManager

LOG = logging.getLogger('pipelined.qos.qos_tc_impl')
# LOG.setLevel(logging.DEBUG)

# TODO - replace this implementation with pyroute2 tc
ROOT_QID = 65534
Expand All @@ -35,7 +35,7 @@ class TrafficClass:
Creates/Deletes queues in linux. Using Qdiscs for flow based
rate limiting(traffic shaping) of user traffic.
"""
tc_ops = None
tc_ops: Optional[TcOpsBase] = None

@staticmethod
def delete_class(intf: str, qid: int, skip_filter=False) -> int:
Expand Down
2 changes: 1 addition & 1 deletion lte/gateway/python/magma/pipelined/qos/tc_ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class TcOpsBase(ABC):

@abstractmethod
def create_htb(
self, iface: str, qid: str, max_bw: str, rate: str,
self, iface: str, qid: str, max_bw: int, rate: str,
parent_qid: str = None,
) -> int:
"""
Expand Down
2 changes: 1 addition & 1 deletion lte/gateway/python/magma/pipelined/qos/tc_ops_cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def __init__(self):
LOG.info("initialized")

def create_htb(
self, iface: str, qid: str, max_bw: str, rate: str,
self, iface: str, qid: str, max_bw: int, rate: str,
parent_qid: str = None,
) -> int:
tc_cmd = "tc class add dev {intf} parent {parent_qid} "
Expand Down
2 changes: 1 addition & 1 deletion lte/gateway/python/magma/pipelined/tests/pipelined_test_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ def __exit__(self, type, value, traceback):
include_stats=self._include_stats,
)
except WaitTimeExceeded as e:
ofctl_cmd = "sudo ovs-ofctl dump-flows %s".format(self._bridge_name)
ofctl_cmd = f"sudo ovs-ofctl dump-flows {self._bridge_name}"
p = subprocess.Popen(
[ofctl_cmd],
stdout=subprocess.PIPE,
Expand Down
2 changes: 0 additions & 2 deletions lte/gateway/python/magma/pipelined/tests/test_ebpf_ul_dp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,12 @@ def tearDownClassDevices(cls):

@classmethod
def pkt_cap_fun(cls, packet):
# print("got packet: %s", packet)
cls.packet_cap1.append(packet)

@classmethod
def count_udp_packet(cls):
cnt = 0
for pkt in cls.packet_cap1:
# print(pkt.show(dump=True))
if IP in pkt:
if pkt[IP].src == cls.inner_src_ip and pkt[IP].dst == cls.inner_dst_ip:
cnt = cnt + 1
Expand Down
1 change: 0 additions & 1 deletion lte/gateway/python/magma/pipelined/tests/test_qos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@ def _testMultipleSubscribers(
exp_id = id_list[i]
k = get_key_json(get_subscriber_key(imsi, '', rule_num, d))
exp_id_dict[k] = exp_id
# self.assertTrue(qos_mgr._redis_store[k] == exp_id)
qid_info = get_data_json(get_subscriber_data(exp_id, 0, 0))
self.assertEqual(qos_mgr._subscriber_state[imsi].rules[rule_num][0], (d, qid_info))

Expand Down
1 change: 0 additions & 1 deletion lte/gateway/python/magma/pipelined/tests/test_qos_pyroute2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ def check_qid_in_tc(self, qid):
ln = ln.strip()
if not ln:
continue
# print(ln)
tokens = ln.split(" ")

if len(tokens) > 11 and tokens[11] == qid:
Expand Down