On Tue, 1 Sep 2020 14:51:45 -0700 David Awogbemila wrote:
> @@ -297,6 +317,22 @@ static inline void gve_clear_probe_in_progress(struct
> gve_priv *priv)
> clear_bit(GVE_PRIV_FLAGS_PROBE_IN_PROGRESS, &priv->service_task_flags);
> }
>
> +static inline bool gve_get_do_report_stats(struct gve_priv *priv)
> +{
> + return test_bit(GVE_PRIV_FLAGS_DO_REPORT_STATS,
> + &priv->service_task_flags);
> +}
> +
> +static inline void gve_set_do_report_stats(struct gve_priv *priv)
> +{
> + set_bit(GVE_PRIV_FLAGS_DO_REPORT_STATS, &priv->service_task_flags);
> +}
> +
> +static inline void gve_clear_do_report_stats(struct gve_priv *priv)
> +{
> + clear_bit(GVE_PRIV_FLAGS_DO_REPORT_STATS, &priv->service_task_flags);
> +}
> +
> static inline bool gve_get_admin_queue_ok(struct gve_priv *priv)
> {
> return test_bit(GVE_PRIV_FLAGS_ADMIN_QUEUE_OK, &priv->state_flags);
> @@ -357,6 +393,21 @@ static inline void gve_clear_napi_enabled(struct
> gve_priv *priv)
> clear_bit(GVE_PRIV_FLAGS_NAPI_ENABLED, &priv->state_flags);
> }
>
> +static inline bool gve_get_report_stats(struct gve_priv *priv)
> +{
> + return test_bit(GVE_PRIV_FLAGS_REPORT_STATS, &priv->ethtool_flags);
> +}
> +
> +static inline void gve_set_report_stats(struct gve_priv *priv)
Please remove the unused helpers.
> +{
> + set_bit(GVE_PRIV_FLAGS_REPORT_STATS, &priv->ethtool_flags);
> +}
> +
> +static inline void gve_clear_report_stats(struct gve_priv *priv)
> +{
> + clear_bit(GVE_PRIV_FLAGS_REPORT_STATS, &priv->ethtool_flags);
> +}
> @@ -353,6 +377,54 @@ static int gve_set_tunable(struct net_device *netdev,
> }
> }
>
> +static u32 gve_get_priv_flags(struct net_device *netdev)
> +{
> + struct gve_priv *priv = netdev_priv(netdev);
> + u32 i, ret_flags = 0;
> +
> + for (i = 0; i < GVE_PRIV_FLAGS_STR_LEN; i++) {
Please remove this pointless loop.
> + if (priv->ethtool_flags & BIT(i))
> + ret_flags |= BIT(i);
> + }
> + return ret_flags;
> +}
> +
> +static int gve_set_priv_flags(struct net_device *netdev, u32 flags)
> +{
> + struct gve_priv *priv = netdev_priv(netdev);
> + u64 ori_flags, new_flags;
> + u32 i;
> +
> + ori_flags = READ_ONCE(priv->ethtool_flags);
> + new_flags = ori_flags;
> +
> + for (i = 0; i < GVE_PRIV_FLAGS_STR_LEN; i++) {
Ditto.
> + if (flags & BIT(i))
> + new_flags |= BIT(i);
> + else
> + new_flags &= ~(BIT(i));
> + priv->ethtool_flags = new_flags;
> + /* set report-stats */
> + if (strcmp(gve_gstrings_priv_flags[i], "report-stats") == 0) {
> + /* update the stats when user turns report-stats on */
> + if (flags & BIT(i))
> + gve_handle_report_stats(priv);
> + /* zero off gve stats when report-stats turned off */
> + if (!(flags & BIT(i)) && (ori_flags & BIT(i))) {
> + int tx_stats_num = GVE_TX_STATS_REPORT_NUM *
> + priv->tx_cfg.num_queues;
> + int rx_stats_num = GVE_RX_STATS_REPORT_NUM *
> + priv->rx_cfg.num_queues;
new line here
> + memset(priv->stats_report->stats, 0,
> + (tx_stats_num + rx_stats_num) *
> + sizeof(struct stats));
> + }
> + }
> + }
> +
> + return 0;
> +}
> +static int gve_alloc_stats_report(struct gve_priv *priv)
> +{
> + int tx_stats_num, rx_stats_num;
> +
> + tx_stats_num = (GVE_TX_STATS_REPORT_NUM) *
> + priv->tx_cfg.num_queues;
> + rx_stats_num = (GVE_RX_STATS_REPORT_NUM) *
> + priv->rx_cfg.num_queues;
> + priv->stats_report_len = sizeof(struct gve_stats_report) +
> + (tx_stats_num + rx_stats_num) *
> + sizeof(struct stats);
> + priv->stats_report =
> + dma_alloc_coherent(&priv->pdev->dev, priv->stats_report_len,
> + &priv->stats_report_bus, GFP_KERNEL);
> + if (!priv->stats_report)
> + return -ENOMEM;
> + /* Set up timer for periodic task */
> + timer_setup(&priv->service_timer, gve_service_timer, 0);
> + priv->service_timer_period = GVE_SERVICE_TIMER_PERIOD;
> + /* Start the service task timer */
> + mod_timer(&priv->service_timer,
> + round_jiffies(jiffies +
> + msecs_to_jiffies(priv->service_timer_period)));
> + return 0;
> +}
> @@ -1173,6 +1315,7 @@ static int gve_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> priv->db_bar2 = db_bar;
> priv->service_task_flags = 0x0;
> priv->state_flags = 0x0;
> + priv->ethtool_flags = 0x0;
> priv->dma_mask = dma_mask;
You allocate the memory and start the timer even tho the priv flag
defaults to off?
