| Message ID | 20200424082313.7666-5-sriharsha.basavapatna@broadcom.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RFC for Partial action offload | expand |
On 4/24/2020 11:23 AM, Sriharsha Basavapatna wrote: > For flows that offload partial actions in egress direction, > provide the right netdev to fetch statistics. > > Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> > --- > lib/dpif-netdev.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 7fcc0b06d..bfe454eb0 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -3199,8 +3199,14 @@ dpif_netdev_get_flow_offload_status(const struct dp_netdev *dp, > return false; > } > > - netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, > - dpif_normalize_type(dp->class->type)); > + if (netdev_flow->partial_actions_offloaded && > + netdev_flow->egress_offload_port != ODPP_NONE) { > + netdev = netdev_ports_get(netdev_flow->egress_offload_port, > + dpif_normalize_type(dp->class->type)); > + } else { > + netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, > + dpif_normalize_type(dp->class->type)); > + } As I see, the egress rules have "count" action. It means that packets are counted twice - both in SW and in HW. However, note that if they don't have the count action, netdev_flow_get will fail and dpctl/dump-flows won't show "partial". > if (!netdev) { > return false; > }
On Wed, May 6, 2020 at 5:56 PM Eli Britstein <elibr@mellanox.com> wrote: > > > On 4/24/2020 11:23 AM, Sriharsha Basavapatna wrote: > > For flows that offload partial actions in egress direction, > > provide the right netdev to fetch statistics. > > > > Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> > > --- > > lib/dpif-netdev.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index 7fcc0b06d..bfe454eb0 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -3199,8 +3199,14 @@ dpif_netdev_get_flow_offload_status(const struct dp_netdev *dp, > > return false; > > } > > > > - netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, > > - dpif_normalize_type(dp->class->type)); > > + if (netdev_flow->partial_actions_offloaded && > > + netdev_flow->egress_offload_port != ODPP_NONE) { > > + netdev = netdev_ports_get(netdev_flow->egress_offload_port, > > + dpif_normalize_type(dp->class->type)); > > + } else { > > + netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, > > + dpif_normalize_type(dp->class->type)); > > + } > > As I see, the egress rules have "count" action. It means that packets > are counted twice - both in SW and in HW. [Harsha] I agree, count action is not needed with partial action offload. I'll fix this. I probably didn't see this issue since I was using a pmd with count action processing disabled. > > However, note that if they don't have the count action, netdev_flow_get > will fail and dpctl/dump-flows won't show "partial". [Harsha] No it won't fail and dpctl/dump-flows shows "partial". This is already handled in Patch-2 where we set "actions_offloaded = false" with partial action offload (see comments there). > > > if (!netdev) { > > return false; > > }
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 7fcc0b06d..bfe454eb0 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -3199,8 +3199,14 @@ dpif_netdev_get_flow_offload_status(const struct dp_netdev *dp, return false; } - netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, - dpif_normalize_type(dp->class->type)); + if (netdev_flow->partial_actions_offloaded && + netdev_flow->egress_offload_port != ODPP_NONE) { + netdev = netdev_ports_get(netdev_flow->egress_offload_port, + dpif_normalize_type(dp->class->type)); + } else { + netdev = netdev_ports_get(netdev_flow->flow.in_port.odp_port, + dpif_normalize_type(dp->class->type)); + } if (!netdev) { return false; }
For flows that offload partial actions in egress direction, provide the right netdev to fetch statistics. Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com> --- lib/dpif-netdev.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)