Skip to content

Commit ca5e99c

Browse files
JBYoshiJonathanWoollett-Light
authored andcommitted
[Test code] Change packet lengths to sized types
IPv4 packet header sizes fit in a u8 value, and all IP/Ethernet-related packet total sizes fit in a u16 value. This changes those sizes to use smaller types that don't ever need to be converted to smaller types. Signed-off-by: Jonathan Browne <[email protected]>
1 parent 3ca5b7a commit ca5e99c

File tree

5 files changed

+47
-33
lines changed

5 files changed

+47
-33
lines changed

src/vmm/src/dumbo/pdu/ipv4.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ mod tests {
471471
use super::*;
472472
use crate::dumbo::MacAddr;
473473

474-
const MAX_HEADER_LEN: usize = 60;
474+
const MAX_HEADER_LEN: u8 = 60;
475475

476476
#[test]
477477
fn test_set_get() {
@@ -532,12 +532,16 @@ mod tests {
532532
let buf_len = buf.len();
533533
// No IPv4 option support for now.
534534
let header_len = OPTIONS_OFFSET;
535-
let payload_len = buf_len - OPTIONS_OFFSET;
535+
let payload_len = buf_len - usize::from(OPTIONS_OFFSET);
536536

537537
{
538538
let mut p = IPv4Packet::write_header(buf.as_mut(), PROTOCOL_TCP, src, dst)
539539
.unwrap()
540-
.with_header_and_payload_len_unchecked(header_len, payload_len, true);
540+
.with_header_and_payload_len_unchecked(
541+
header_len,
542+
u16::try_from(payload_len).unwrap(),
543+
true,
544+
);
541545

542546
assert_eq!(p.version_and_header_len(), (IPV4_VERSION, header_len));
543547
assert_eq!(p.dscp_and_ecn(), (0, 0));
@@ -595,7 +599,7 @@ mod tests {
595599
// Total length smaller than header length.
596600
p(buf.as_mut())
597601
.set_version_and_header_len(IPV4_VERSION, OPTIONS_OFFSET)
598-
.set_total_len(OPTIONS_OFFSET as u16 - 1);
602+
.set_total_len(u16::from(OPTIONS_OFFSET) - 1);
599603
look_for_error(buf.as_ref(), Error::InvalidTotalLen);
600604

601605
// Total len not matching slice length.
@@ -643,7 +647,7 @@ mod tests {
643647

644648
assert_eq!(p.compute_checksum(), 0);
645649
assert_eq!(p.total_len() as usize, p.len());
646-
assert_eq!(p.len(), header_len + payload_len);
650+
assert_eq!(p.len(), usize::from(header_len) + usize::from(payload_len));
647651
}
648652

649653
{
@@ -653,7 +657,7 @@ mod tests {
653657

654658
assert_eq!(p.compute_checksum(), 0);
655659
assert_eq!(p.total_len() as usize, p.len());
656-
assert_eq!(p.len(), header_len + payload_len);
660+
assert_eq!(p.len(), usize::from(header_len) + usize::from(payload_len));
657661
}
658662
}
659663

src/vmm/src/dumbo/pdu/tcp.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,7 @@ mod tests {
651651
let mss_option = Some(mss_left);
652652
let payload = Some((b.as_ref(), b.len()));
653653

654-
let header_len = OPTIONS_OFFSET + OPTION_LEN_MSS.into();
654+
let header_len = OPTIONS_OFFSET + OPTION_LEN_MSS;
655655

656656
let segment_len = {
657657
let mut segment = TcpSegment::write_segment(
@@ -688,24 +688,28 @@ mod tests {
688688
assert_eq!(segment.urgent_pointer(), 0);
689689

690690
{
691-
let options = segment.options_unchecked(header_len);
692-
assert_eq!(options.len(), OPTION_LEN_MSS.into());
691+
let options = segment.options_unchecked(header_len.into());
692+
assert_eq!(options.len(), usize::from(OPTION_LEN_MSS));
693693
assert_eq!(options[0], OPTION_KIND_MSS);
694694
assert_eq!(options[1], OPTION_LEN_MSS);
695695
assert_eq!(options.ntohs_unchecked(2), mss_left);
696696
}
697697

698698
// Payload was smaller than mss_left after options.
699-
assert_eq!(segment.len(), header_len + b.len());
699+
assert_eq!(
700+
usize::from(segment.len()),
701+
usize::from(header_len) + b.len(),
702+
);
700703
segment.len()
701704
// Mutable borrow of a goes out of scope.
702705
};
703706

704707
{
705708
let segment =
706-
TcpSegment::from_bytes(&a[..segment_len], Some((src_addr, dst_addr))).unwrap();
709+
TcpSegment::from_bytes(&a[..segment_len.into()], Some((src_addr, dst_addr)))
710+
.unwrap();
707711
assert_eq!(
708-
segment.parse_mss_option_unchecked(header_len),
712+
segment.parse_mss_option_unchecked(header_len.into()),
709713
Ok(Some(NonZeroU16::new(mss_left).unwrap()))
710714
);
711715
}
@@ -728,7 +732,7 @@ mod tests {
728732
.unwrap()
729733
.len();
730734

731-
assert_eq!(segment_len, mss_left as usize);
735+
assert_eq!(segment_len, mss_left);
732736
}
733737

734738
// Now let's test the error value for from_bytes().

src/vmm/src/dumbo/tcp/connection.rs

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,7 @@ pub(crate) mod tests {
11101110
data_buf: &[u8],
11111111
) -> TcpSegment<'a, &'a mut [u8]> {
11121112
let segment = self.write_segment_helper(buf, false, Some((data_buf, data_buf.len())));
1113-
assert_eq!(segment.payload_len(), data_buf.len());
1113+
assert_eq!(usize::from(segment.payload_len()), data_buf.len());
11141114
segment
11151115
}
11161116

@@ -1203,7 +1203,7 @@ pub(crate) mod tests {
12031203
options_len: usize,
12041204
flags_after_ns: TcpFlags,
12051205
) {
1206-
assert_eq!(s.len(), BASIC_SEGMENT_SIZE + options_len);
1206+
assert_eq!(usize::from(s.len()), BASIC_SEGMENT_SIZE + options_len);
12071207
assert_eq!(s.flags_after_ns(), flags_after_ns);
12081208
}
12091209

@@ -1471,13 +1471,13 @@ pub(crate) mod tests {
14711471
assert_eq!(
14721472
t.receive_segment(&mut c, &data).unwrap(),
14731473
(
1474-
Some(NonZeroUsize::new(data.payload_len()).unwrap()),
1474+
Some(NonZeroUsize::new(data.payload_len().into()).unwrap()),
14751475
RecvStatusFlags::empty()
14761476
)
14771477
);
14781478

14791479
// This is the ack number that should be set/sent.
1480-
let expected_ack = t.remote_isn.wrapping_add(data.payload_len() as u32 + 1);
1480+
let expected_ack = t.remote_isn.wrapping_add(u32::from(data.payload_len()) + 1);
14811481

14821482
// Check that internal state gets updated properly.
14831483
assert_eq!(c.ack_to_send.0, expected_ack);
@@ -1493,7 +1493,7 @@ pub(crate) mod tests {
14931493
assert!(t.write_next_segment(&mut c, None).unwrap().is_none());
14941494

14951495
{
1496-
let payload_len = data.payload_len() as u32;
1496+
let payload_len = u32::from(data.payload_len());
14971497

14981498
// Assuming no one changed the code, the local window size of the connection was 10000,
14991499
// so we should be able to successfully receive 9 more segments with 1000 byte payloads.
@@ -1504,7 +1504,7 @@ pub(crate) mod tests {
15041504
assert_eq!(
15051505
t.receive_segment(&mut c, &data).unwrap(),
15061506
(
1507-
Some(NonZeroUsize::new(data.payload_len()).unwrap()),
1507+
Some(NonZeroUsize::new(data.payload_len().into()).unwrap()),
15081508
RecvStatusFlags::empty()
15091509
)
15101510
);
@@ -1550,7 +1550,7 @@ pub(crate) mod tests {
15501550
// The mss is 1100, and the remote window is 11000, so we can send 10 data packets.
15511551
let max = 10;
15521552
let remote_isn = t.remote_isn;
1553-
let mss = u32::from(t.mss);
1553+
let mss = t.mss;
15541554

15551555
let (payload_buf, mut response_seq) = payload_src.unwrap();
15561556
let mut payload_offset = 0;
@@ -1561,22 +1561,25 @@ pub(crate) mod tests {
15611561
.unwrap_or_else(|_| panic!("{}", i))
15621562
.unwrap_or_else(|| panic!("{}", i));
15631563

1564-
payload_offset += s.payload_len();
1565-
response_seq += Wrapping(s.payload_len() as u32);
1564+
payload_offset += usize::from(s.payload_len());
1565+
response_seq += Wrapping(u32::from(s.payload_len()));
15661566

15671567
// Again, the 1 accounts for the sequence number taken up by the SYN.
1568-
assert_eq!(s.sequence_number(), conn_isn.wrapping_add(1 + i * mss));
1568+
assert_eq!(
1569+
s.sequence_number(),
1570+
conn_isn.wrapping_add(1 + i * u32::from(mss)),
1571+
);
15691572
assert_eq!(s.ack_number(), remote_isn.wrapping_add(1));
15701573
assert_eq!(s.flags_after_ns(), TcpFlags::ACK);
1571-
assert_eq!(s.payload_len() as u32, mss);
1574+
assert_eq!(s.payload_len(), mss);
15721575
}
15731576

15741577
// No more new data can be sent until the window advances, even though data_buf
15751578
// contains 20_000 bytes.
15761579
assert!(t.write_next_segment(&mut c, payload_src).unwrap().is_none());
15771580

15781581
// Let's ACK the first segment previously sent.
1579-
ctrl.set_ack_number(conn_isn.wrapping_add(1 + mss))
1582+
ctrl.set_ack_number(conn_isn.wrapping_add(1 + u32::from(mss)))
15801583
.set_flags_after_ns(TcpFlags::ACK);
15811584
assert_eq!(
15821585
t.receive_segment(&mut c, &ctrl).unwrap(),
@@ -1586,8 +1589,11 @@ pub(crate) mod tests {
15861589
// We should be able to send one more segment now.
15871590
{
15881591
let s = t.write_next_segment(&mut c, payload_src).unwrap().unwrap();
1589-
assert_eq!(s.sequence_number(), conn_isn.wrapping_add(1 + max * mss));
1590-
assert_eq!(s.payload_len(), mss as usize);
1592+
assert_eq!(
1593+
s.sequence_number(),
1594+
conn_isn.wrapping_add(1 + max * u32::from(mss)),
1595+
);
1596+
assert_eq!(s.payload_len(), mss);
15911597
}
15921598
assert!(t.write_next_segment(&mut c, payload_src).unwrap().is_none());
15931599

@@ -1604,7 +1610,7 @@ pub(crate) mod tests {
16041610
{
16051611
let s = t.write_next_segment(&mut c, payload_src).unwrap().unwrap();
16061612
assert_eq!(s.sequence_number(), ctrl.ack_number());
1607-
assert_eq!(s.payload_len(), mss as usize);
1613+
assert_eq!(s.payload_len(), mss);
16081614
}
16091615
assert!(t.write_next_segment(&mut c, payload_src).unwrap().is_none());
16101616

@@ -1613,7 +1619,7 @@ pub(crate) mod tests {
16131619
{
16141620
let s = t.write_next_segment(&mut c, payload_src).unwrap().unwrap();
16151621
assert_eq!(s.sequence_number(), ctrl.ack_number());
1616-
assert_eq!(s.payload_len(), mss as usize);
1622+
assert_eq!(s.payload_len(), mss);
16171623
}
16181624
assert!(t.write_next_segment(&mut c, payload_src).unwrap().is_none());
16191625

@@ -1626,7 +1632,7 @@ pub(crate) mod tests {
16261632
{
16271633
let s = t.write_next_segment(&mut c, payload_src).unwrap().unwrap();
16281634
assert_eq!(s.sequence_number(), ctrl.ack_number());
1629-
assert_eq!(s.payload_len(), mss as usize);
1635+
assert_eq!(s.payload_len(), mss);
16301636
}
16311637
assert!(t.write_next_segment(&mut c, payload_src).unwrap().is_none());
16321638

src/vmm/src/dumbo/tcp/endpoint.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ mod tests {
490490
endpoint_first_not_sent = s
491491
.inner()
492492
.sequence_number()
493-
.wrapping_add(s.inner().payload_len() as u32);
493+
.wrapping_add(u32::from(s.inner().payload_len()));
494494
}
495495

496496
// Cool, now let's check that even though receive_buf is limited to some value, we can
@@ -535,7 +535,7 @@ mod tests {
535535
assert!(response.contains("200"));
536536

537537
endpoint_first_not_sent =
538-
endpoint_first_not_sent.wrapping_add(s.inner().payload_len() as u32);
538+
endpoint_first_not_sent.wrapping_add(u32::from(s.inner().payload_len()));
539539
}
540540
}
541541

src/vmm/src/dumbo/tcp/handler.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ mod tests {
544544
(p.header_len(), p.len())
545545
};
546546

547-
TcpSegment::from_bytes(&mut buf[segment_start..segment_end], None).unwrap()
547+
TcpSegment::from_bytes(&mut buf[segment_start.into()..segment_end], None).unwrap()
548548
}
549549

550550
// Calls write_next_packet until either an error occurs, or there's nothing left to send.

0 commit comments

Comments
 (0)