On Tue, Sep 23, 2025 at 09:19:17AM -0500, Daniel Jurgens wrote:
> Add support for IP_USER type rules from ethtool.
> 
> Example:
> $ ethtool -U ens9 flow-type ip4 src-ip 192.168.51.101 action -1
> Added rule with ID 1
> 
> The example rule will drop packets with the source IP specified.
> 
> Signed-off-by: Daniel Jurgens <dani...@nvidia.com>
> Reviewed-by: Parav Pandit <pa...@nvidia.com>
> Reviewed-by: Shahar Shitrit <shshit...@nvidia.com>
> ---
>  drivers/net/virtio_net/virtio_net_ff.c | 127 +++++++++++++++++++++++--
>  1 file changed, 119 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/virtio_net/virtio_net_ff.c 
> b/drivers/net/virtio_net/virtio_net_ff.c
> index 30c5ded57ab5..0374676d1342 100644
> --- a/drivers/net/virtio_net/virtio_net_ff.c
> +++ b/drivers/net/virtio_net/virtio_net_ff.c
> @@ -90,6 +90,34 @@ static bool validate_eth_mask(const struct virtnet_ff *ff,
>       return true;
>  }
>  
> +static bool validate_ip4_mask(const struct virtnet_ff *ff,
> +                           const struct virtio_net_ff_selector *sel,
> +                           const struct virtio_net_ff_selector *sel_cap)

I'd prefer that all functions have virtnet prefix,
avoid polluting the global namespace.


> +{
> +     bool partial_mask = !!(sel_cap->flags & 
> VIRTIO_NET_FF_MASK_F_PARTIAL_MASK);
> +     struct iphdr *cap, *mask;
> +
> +     cap = (struct iphdr *)&sel_cap->mask;
> +     mask = (struct iphdr *)&sel->mask;
> +
> +     if (mask->saddr &&
> +         !check_mask_vs_cap(&mask->saddr, &cap->saddr,
> +         sizeof(__be32), partial_mask))


pls align continuation to the right of (.

> +             return false;
> +
> +     if (mask->daddr &&
> +         !check_mask_vs_cap(&mask->daddr, &cap->daddr,
> +         sizeof(__be32), partial_mask))


and here

> +             return false;
> +
> +     if (mask->protocol &&
> +         !check_mask_vs_cap(&mask->protocol, &cap->protocol,
> +         sizeof(u8), partial_mask))


and here


> +             return false;
> +
> +     return true;
> +}
> +
>  static bool validate_mask(const struct virtnet_ff *ff,
>                         const struct virtio_net_ff_selector *sel)
>  {
> @@ -101,11 +129,36 @@ static bool validate_mask(const struct virtnet_ff *ff,
>       switch (sel->type) {
>       case VIRTIO_NET_FF_MASK_TYPE_ETH:
>               return validate_eth_mask(ff, sel, sel_cap);
> +
> +     case VIRTIO_NET_FF_MASK_TYPE_IPV4:
> +             return validate_ip4_mask(ff, sel, sel_cap);
>       }
>  
>       return false;
>  }
>  
> +static void parse_ip4(struct iphdr *mask, struct iphdr *key,
> +                   const struct ethtool_rx_flow_spec *fs)
> +{
> +     const struct ethtool_usrip4_spec *l3_mask = &fs->m_u.usr_ip4_spec;
> +     const struct ethtool_usrip4_spec *l3_val  = &fs->h_u.usr_ip4_spec;
> +
> +     mask->saddr = l3_mask->ip4src;
> +     mask->daddr = l3_mask->ip4dst;
> +     key->saddr = l3_val->ip4src;
> +     key->daddr = l3_val->ip4dst;
> +
> +     if (mask->protocol) {
> +             mask->protocol = l3_mask->proto;

Is this right? You just checked mask->protocol and are
now overriding it?


> +             key->protocol = l3_val->proto;
> +     }
> +}
> +
> +static bool has_ipv4(u32 flow_type)
> +{
> +     return flow_type == IP_USER_FLOW;
> +}
> +
>  static int setup_classifier(struct virtnet_ff *ff,
>                           struct virtnet_classifier **c)
>  {
> @@ -237,6 +290,7 @@ static bool supported_flow_type(const struct 
> ethtool_rx_flow_spec *fs)
>  {
>       switch (fs->flow_type) {
>       case ETHER_FLOW:
> +     case IP_USER_FLOW:
>               return true;
>       }
>  
> @@ -260,16 +314,27 @@ static int validate_flow_input(struct virtnet_ff *ff,
>  
>       if (!supported_flow_type(fs))
>               return -EOPNOTSUPP;
> -
>       return 0;
>  }
>  
>  static void calculate_flow_sizes(struct ethtool_rx_flow_spec *fs,
> -                              size_t *key_size, size_t *classifier_size,
> -                              int *num_hdrs)
> +                             size_t *key_size, size_t *classifier_size,
> +                             int *num_hdrs)
>  {
> +     size_t size = sizeof(struct ethhdr);
> +
>       *num_hdrs = 1;
>       *key_size = sizeof(struct ethhdr);
> +
> +     if (fs->flow_type == ETHER_FLOW)
> +             goto done;
> +
> +     (*num_hdrs)++;

I prefer ++(*num_hdrs) in such cases generally. why return old value if
we discard it anyway?

> +     if (has_ipv4(fs->flow_type))
> +             size += sizeof(struct iphdr);
> +
> +done:
> +     *key_size = size;
>       /*
>        * The classifier size is the size of the classifier header, a selector
>        * header for each type of header in the match criteria, and each header
> @@ -281,8 +346,9 @@ static void calculate_flow_sizes(struct 
> ethtool_rx_flow_spec *fs,
>  }
>  
>  static void setup_eth_hdr_key_mask(struct virtio_net_ff_selector *selector,
> -                                u8 *key,
> -                                const struct ethtool_rx_flow_spec *fs)
> +                               u8 *key,
> +                               const struct ethtool_rx_flow_spec *fs,
> +                               int num_hdrs)
>  {
>       struct ethhdr *eth_m = (struct ethhdr *)&selector->mask;
>       struct ethhdr *eth_k = (struct ethhdr *)key;
> @@ -290,8 +356,33 @@ static void setup_eth_hdr_key_mask(struct 
> virtio_net_ff_selector *selector,
>       selector->type = VIRTIO_NET_FF_MASK_TYPE_ETH;
>       selector->length = sizeof(struct ethhdr);
>  
> -     memcpy(eth_m, &fs->m_u.ether_spec, sizeof(*eth_m));
> -     memcpy(eth_k, &fs->h_u.ether_spec, sizeof(*eth_k));
> +     if (num_hdrs > 1) {
> +             eth_m->h_proto = cpu_to_be16(0xffff);
> +             eth_k->h_proto = cpu_to_be16(ETH_P_IP);
> +     } else {
> +             memcpy(eth_m, &fs->m_u.ether_spec, sizeof(*eth_m));
> +             memcpy(eth_k, &fs->h_u.ether_spec, sizeof(*eth_k));
> +     }
> +}
> +
> +static int setup_ip_key_mask(struct virtio_net_ff_selector *selector,
> +                          u8 *key,
> +                          const struct ethtool_rx_flow_spec *fs)
> +{
> +     struct iphdr *v4_m = (struct iphdr *)&selector->mask;
> +     struct iphdr *v4_k = (struct iphdr *)key;
> +
> +     selector->type = VIRTIO_NET_FF_MASK_TYPE_IPV4;
> +     selector->length = sizeof(struct iphdr);
> +
> +     if (fs->h_u.usr_ip4_spec.l4_4_bytes ||
> +         fs->h_u.usr_ip4_spec.tos ||
> +         fs->h_u.usr_ip4_spec.ip_ver != ETH_RX_NFC_IP4)
> +             return -EOPNOTSUPP;
> +
> +     parse_ip4(v4_m, v4_k, fs);
> +
> +     return 0;
>  }
>  
>  static int
> @@ -312,6 +403,17 @@ validate_classifier_selectors(struct virtnet_ff *ff,
>       return 0;
>  }
>  
> +static
> +struct virtio_net_ff_selector *next_selector(struct virtio_net_ff_selector 
> *sel)
> +{
> +     void *nextsel;
> +
> +     nextsel = (u8 *)sel + sizeof(struct virtio_net_ff_selector) +
> +               sel->length;

you do not need this variable. and cast to void* looks cleaner imho.

> +
> +     return nextsel;
> +}
> +
>  static int build_and_insert(struct virtnet_ff *ff,
>                           struct virtnet_ethtool_rule *eth_rule)
>  {
> @@ -349,8 +451,17 @@ static int build_and_insert(struct virtnet_ff *ff,
>       classifier->count = num_hdrs;
>       selector = &classifier->selectors[0];
>  
> -     setup_eth_hdr_key_mask(selector, key, fs);
> +     setup_eth_hdr_key_mask(selector, key, fs, num_hdrs);
> +     if (num_hdrs == 1)
> +             goto validate;
> +
> +     selector = next_selector(selector);
> +
> +     err = setup_ip_key_mask(selector, key + sizeof(struct ethhdr), fs);
> +     if (err)
> +             goto err_classifier;
>  
> +validate:
>       err = validate_classifier_selectors(ff, classifier, num_hdrs);
>       if (err)
>               goto err_key;
> -- 
> 2.45.0


Reply via email to