On 3/1/2023 8:58 PM, Ilya Maximets wrote:
On 2/28/23 14:17, Chris Mi wrote:
On 2/24/2023 4:46 AM, Ilya Maximets wrote:
On 2/23/23 12:27, Chris Mi wrote:
Create a unique group ID to map the sFlow info when offloading sample
action to TC. When showing the offloaded datapath flows, translate the
group ID from TC sample action to sFlow info using the mapping.

Signed-off-by: Chris Mi<[email protected]>
Reviewed-by: Roi Dayan<[email protected]>
---
  lib/netdev-offload-tc.c | 244 +++++++++++++++++++++++++++++++++++++---
  lib/netdev-offload.h    |   9 ++
  lib/tc.c                |  62 +++++++++-
  lib/tc.h                |  11 +-
  4 files changed, 309 insertions(+), 17 deletions(-)

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index d7901fa68..ec4813147 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -2099,6 +2118,172 @@ parse_match_ct_state_to_flower(struct tc_flower 
*flower, struct match *match)
      }
  }
+static int
+parse_userspace_attributes(const struct nlattr *actions,
+                           struct offload_sflow *sflow)
+{
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+    const struct nlattr *nla;
+    unsigned int left;
+
+    NL_NESTED_FOR_EACH_UNSAFE (nla, left, actions) {
+        if (nl_attr_type(nla) == OVS_USERSPACE_ATTR_USERDATA) {
+            struct user_action_cookie *cookie;
+
+            cookie = (struct user_action_cookie *) nl_attr_get(nla);
+            if (cookie->type == USER_ACTION_COOKIE_SFLOW) {
This pointer is likely misaligned.
But the cookie is passed from ofproto. I believe it is aligned when allocating 
it.
Netlink attributes are always only 4-byte aligned.  So, it could have
been misaligned if struct user_action_cookie had a higher alignment
requirement.  Fortunately, it doesn't look like it.
OK.

diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index e332555ee..fb35d77c4 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -81,6 +81,15 @@ struct offload_info {
      bool tc_modify_flow_deleted; /* Indicate the tc modify flow put success
                                    * to delete the original flow. */
      odp_port_t orig_in_port; /* Originating in_port for tnl flows. */
+
+    /* To avoid passing too many parameters when offloading sample action,
+     * aggregate them here.
+     */
+    struct {
+        const struct flow_tnl *tnl; /* Tunnel info. */
+        const ovs_u128 *ufid;       /* Ufid. */
+        uint32_t gid;               /* Generated gid. */
+    } sample;
I'm not sure why this structure exists.  It is used internally
in netdev-offload-tc, so should not be public.

The offload_info structure is for communication between layers,
not to store the local information.
I removed all sample members in offload_info and introduced struct sample_param.
If it is not ok. I think we have to add more params in several functions that 
already have
too many params.
The point was that they shouldn't be part of struct offload_info.
How they are stored inside netdev-offload-tc is a different story.
(I didn't look at the updated code yet.)
OK.

Thanks for the explanations.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to