Skip to content

Commit

Permalink
events: ovs: Use Option instead of unit enum value.
Browse files Browse the repository at this point in the history 
OVS module currently makes no assumptions on the order of the event
chunks and tries to build the event even if out-of-order pieces are
received. This makes little sense in practice as these chuncks are sent
in the same hook.

Removing that unneeded requirement and assuming the base action event
(OVS_DP_ACTION) will be received before specific action argument events
(e.g: OVS_DP_ACTION_OUTPUT) makes decoding simpler.

This also it avoids requiring a Default version of the event which is
currently implemented using an "undefined" value for enums. This
mechanism is not supported by pyo3.

Also, create a dummy object to avoid having mixed complex unit variants
in enums.

[1] Upstream discussions:
PyO3/pyo3#3582 (comment)
PyO3/pyo3#3749

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
  • Loading branch information
@amorenoz
amorenoz committed Oct 3, 2024
1 parent ecaedc0 commit f732b70
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 202 deletions.
116 changes: 57 additions & 59 deletions retis-events/src/ovs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use super::*;
use crate::{event_section, event_type, Formatter};

///The OVS Event
#[derive(Default, PartialEq)]
#[derive(PartialEq)]
#[event_section("ovs")]
pub struct OvsEvent {
/// Event data
Expand All @@ -23,7 +23,7 @@ impl EventFmt for OvsEvent {

#[event_type]
#[serde(tag = "event_type")]
#[derive(Default, PartialEq)]
#[derive(PartialEq)]
pub enum OvsEventType {
/// Upcall event. It indicates the begining of an upcall. An upcall can have multiple enqueue
/// events.
Expand Down Expand Up @@ -51,10 +51,6 @@ pub enum OvsEventType {
/// Action execution event. It indicates the datapath has executed an action on a packet.
#[serde(rename = "action_execute")]
Action(ActionEvent),

#[serde(rename = "undefined")]
#[default]
Undefined,
}

impl EventFmt for OvsEventType {
Expand All @@ -67,7 +63,6 @@ impl EventFmt for OvsEventType {
RecvUpcall(e) => e,
Operation(e) => e,
Action(e) => e,
Undefined => return write!(f, "?"),
};

disp.event_fmt(f, format)
Expand Down Expand Up @@ -266,7 +261,7 @@ impl EventFmt for RecvUpcallEvent {
pub struct ActionEvent {
/// Action to be executed.
#[serde(flatten)]
pub action: OvsAction,
pub action: Option<OvsAction>,
/// Recirculation id.
pub recirc_id: u32,
/// Queue id used for tracking. None if not tracking or if the output event did not come from
Expand All @@ -284,19 +279,18 @@ impl EventFmt for ActionEvent {
write!(f, "exec")?;

match &self.action {
OvsAction::Unspecified => write!(f, " (unspecified)")?,
OvsAction::Output(a) => write!(f, " oport {}", a.port)?,
OvsAction::Userspace => write!(f, " userspace")?,
OvsAction::Set => write!(f, " tunnel_set")?,
OvsAction::PushVlan => write!(f, " push_vlan")?,
OvsAction::PopVlan => write!(f, " pop_vlan")?,
OvsAction::Sample => write!(f, " sample")?,
OvsAction::Recirc(a) => write!(f, " recirc {:#x}", a.id)?,
OvsAction::Hash => write!(f, " hash")?,
OvsAction::PushMpls => write!(f, " push_mpls")?,
OvsAction::PopMpls => write!(f, " pop_mpls")?,
OvsAction::SetMasked => write!(f, " set_masked")?,
OvsAction::Ct(ct) => {
Some(OvsAction::Output(a)) => write!(f, " oport {}", a.port)?,
Some(OvsAction::Userspace(_)) => write!(f, " userspace")?,
Some(OvsAction::Set(_)) => write!(f, " tunnel_set")?,
Some(OvsAction::PushVlan(_)) => write!(f, " push_vlan")?,
Some(OvsAction::PopVlan(_)) => write!(f, " pop_vlan")?,
Some(OvsAction::Sample(_)) => write!(f, " sample")?,
Some(OvsAction::Recirc(a)) => write!(f, " recirc {:#x}", a.id)?,
Some(OvsAction::Hash(_)) => write!(f, " hash")?,
Some(OvsAction::PushMpls(_)) => write!(f, " push_mpls")?,
Some(OvsAction::PopMpls(_)) => write!(f, " pop_mpls")?,
Some(OvsAction::SetMasked(_)) => write!(f, " set_masked")?,
Some(OvsAction::Ct(ct)) => {
write!(f, " ct zone {}", ct.zone_id)?;

if let Some(nat) = &ct.nat {
Expand Down Expand Up @@ -358,17 +352,18 @@ impl EventFmt for ActionEvent {
write!(f, " {}", flags.join(","))?;
}
}
OvsAction::Trunc => write!(f, " trunc")?,
OvsAction::PushEth => write!(f, " push_eth")?,
OvsAction::PopEth => write!(f, " pop_eth")?,
OvsAction::CtClear => write!(f, " ct_clear")?,
OvsAction::PushNsh => write!(f, " push_nsh")?,
OvsAction::PopNsh => write!(f, " pop_nsh")?,
OvsAction::Meter => write!(f, " meter")?,
OvsAction::Clone => write!(f, " clone")?,
OvsAction::CheckPktLen => write!(f, " check_pkt_len")?,
OvsAction::AddMpls => write!(f, " add_mpls")?,
OvsAction::DecTtl => write!(f, " dec_ttl")?,
Some(OvsAction::Trunc(_)) => write!(f, " trunc")?,
Some(OvsAction::PushEth(_)) => write!(f, " push_eth")?,
Some(OvsAction::PopEth(_)) => write!(f, " pop_eth")?,
Some(OvsAction::CtClear(_)) => write!(f, " ct_clear")?,
Some(OvsAction::PushNsh(_)) => write!(f, " push_nsh")?,
Some(OvsAction::PopNsh(_)) => write!(f, " pop_nsh")?,
Some(OvsAction::Meter(_)) => write!(f, " meter")?,
Some(OvsAction::Clone(_)) => write!(f, " clone")?,
Some(OvsAction::CheckPktLen(_)) => write!(f, " check_pkt_len")?,
Some(OvsAction::AddMpls(_)) => write!(f, " add_mpls")?,
Some(OvsAction::DecTtl(_)) => write!(f, " dec_ttl")?,
None => write!(f, " unspec")?,
}

if let Some(p) = self.queue_id {
Expand All @@ -379,59 +374,62 @@ impl EventFmt for ActionEvent {
}
}

// Adding unit values in an otherwise complex is not supported by pyo3.
// FIXME: Remove when arguments from all actions are implemented.
#[event_type]
#[derive(PartialEq)]
pub struct OvsDummyAction;

#[event_type]
#[serde(tag = "action")]
#[derive(Default, PartialEq)]
#[derive(PartialEq)]
pub enum OvsAction {
#[serde(rename = "unspecified")]
#[default]
Unspecified,
#[serde(rename = "output")]
Output(OvsActionOutput),
#[serde(rename = "userspace")]
Userspace,
Userspace(OvsDummyAction),
#[serde(rename = "set")]
Set,
Set(OvsDummyAction),
#[serde(rename = "push_vlan")]
PushVlan,
PushVlan(OvsDummyAction),
#[serde(rename = "pop_vlan")]
PopVlan,
PopVlan(OvsDummyAction),
#[serde(rename = "sample")]
Sample,
Sample(OvsDummyAction),
#[serde(rename = "recirc")]
Recirc(OvsActionRecirc),
#[serde(rename = "hash")]
Hash,
Hash(OvsDummyAction),
#[serde(rename = "push_mpls")]
PushMpls,
PushMpls(OvsDummyAction),
#[serde(rename = "pop_mpls")]
PopMpls,
PopMpls(OvsDummyAction),
#[serde(rename = "set_masked")]
SetMasked,
SetMasked(OvsDummyAction),
#[serde(rename = "ct")]
Ct(OvsActionCt),
#[serde(rename = "trunc")]
Trunc,
Trunc(OvsDummyAction),
#[serde(rename = "push_eth")]
PushEth,
PushEth(OvsDummyAction),
#[serde(rename = "pop_eth")]
PopEth,
PopEth(OvsDummyAction),
#[serde(rename = "ct_clear")]
CtClear,
CtClear(OvsDummyAction),
#[serde(rename = "push_nsh")]
PushNsh,
PushNsh(OvsDummyAction),
#[serde(rename = "pop_nsh")]
PopNsh,
PopNsh(OvsDummyAction),
#[serde(rename = "meter")]
Meter,
Meter(OvsDummyAction),
#[serde(rename = "clone")]
Clone,
Clone(OvsDummyAction),
#[serde(rename = "check_pkt_len")]
CheckPktLen,
CheckPktLen(OvsDummyAction),
#[serde(rename = "add_mpls")]
AddMpls,
AddMpls(OvsDummyAction),
#[serde(rename = "dec_ttl")]
DecTtl,
DecTtl(OvsDummyAction),
}

/// OVS output action data.
Expand Down Expand Up @@ -555,7 +553,7 @@ mod tests {
r#"{"action":"output","event_type":"action_execute","port":2,"queue_id":1361394472,"recirc_id":0}"#,
OvsEvent {
event: OvsEventType::Action(ActionEvent {
action: OvsAction::Output(OvsActionOutput { port: 2 }),
action: Some(OvsAction::Output(OvsActionOutput { port: 2 })),
recirc_id: 0,
queue_id: Some(1361394472),
}),
Expand Down Expand Up @@ -615,7 +613,7 @@ mod tests {
r#"{"action":"ct","event_type":"action_execute","flags":485,"nat":{"dir":"dst","max_addr":"10.244.1.30","max_port":36900,"min_addr":"10.244.1.3","min_port":36895},"recirc_id":34,"zone_id":20}"#,
OvsEvent {
event: OvsEventType::Action(ActionEvent {
action: OvsAction::Ct(OvsActionCt {
action: Some(OvsAction::Ct(OvsActionCt {
zone_id: 20,
flags: 485,
nat: Some(OvsActionCtNat {
Expand All @@ -625,7 +623,7 @@ mod tests {
min_port: Some(36895),
max_port: Some(36900),
}),
}),
})),
recirc_id: 34,
queue_id: None,
}),
Expand Down
Loading

0 comments on commit f732b70

Please sign in to comment.