On Mon, Nov 20, 2023 at 04:01:42PM +0200, Roger Quadros wrote:
-static int am65_cpsw_configure_taprio(struct net_device *ndev,
- struct am65_cpsw_est *est_new)
+static void am65_cpsw_cp_taprio(struct tc_taprio_qopt_offload *from,
+ struct tc_taprio_qopt_offload *to)
+{
+ int i;
+
+ *to = *from;
+ for (i = 0; i < from->num_entries; i++)
+ to->entries[i] = from->entries[i];
+}
I think I mentioned this before: have you looked at taprio_offload_get()
and taprio_offload_put()?
+
+static int am65_cpsw_taprio_replace(struct net_device *ndev,
+ struct tc_taprio_qopt_offload *taprio)
{
struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ struct netlink_ext_ack *extack = taprio->mqprio.extack;
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
struct am65_cpts *cpts = common->cpts;
int ret = 0, tact = TACT_PROG;
+ struct am65_cpsw_est *est_new;
- am65_cpsw_est_update_state(ndev);
+ if (!netif_running(ndev)) {
+ NL_SET_ERR_MSG_MOD(extack, "interface is down, link speed unknown\n");
The extack message doesn't need a \n.
+ return -ENETDOWN;
+ }
- if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
- am65_cpsw_stop_est(ndev);
- return ret;
+ if (common->pf_p0_rx_ptype_rrobin) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "p0-rx-ptype-rrobin flag conflicts with taprio qdisc\n");
Also here.
+ return -EINVAL;
+ }
+
+ if (port->qos.link_speed == SPEED_UNKNOWN)
+ return -ENOLINK;
+
+ if (taprio->cycle_time_extension) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "cycle time extension not supported");
Here it's ok.
+ return -EOPNOTSUPP;
}
On 21/11/2023 00:56, Vladimir Oltean wrote:
On Mon, Nov 20, 2023 at 04:01:42PM +0200, Roger Quadros wrote:
quoted
-static int am65_cpsw_configure_taprio(struct net_device *ndev,
- struct am65_cpsw_est *est_new)
+static void am65_cpsw_cp_taprio(struct tc_taprio_qopt_offload *from,
+ struct tc_taprio_qopt_offload *to)
+{
+ int i;
+
+ *to = *from;
+ for (i = 0; i < from->num_entries; i++)
+ to->entries[i] = from->entries[i];
+}
I think I mentioned this before: have you looked at taprio_offload_get()
and taprio_offload_put()?
I'm sorry that I missed this. I'll take a look.
quoted
+
+static int am65_cpsw_taprio_replace(struct net_device *ndev,
+ struct tc_taprio_qopt_offload *taprio)
{
struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ struct netlink_ext_ack *extack = taprio->mqprio.extack;
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
struct am65_cpts *cpts = common->cpts;
int ret = 0, tact = TACT_PROG;
+ struct am65_cpsw_est *est_new;
- am65_cpsw_est_update_state(ndev);
+ if (!netif_running(ndev)) {
+ NL_SET_ERR_MSG_MOD(extack, "interface is down, link speed unknown\n");
The extack message doesn't need a \n.
OK.
quoted
+ return -ENETDOWN;
+ }
- if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
- am65_cpsw_stop_est(ndev);
- return ret;
+ if (common->pf_p0_rx_ptype_rrobin) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "p0-rx-ptype-rrobin flag conflicts with taprio qdisc\n");
Also here.
quoted
+ return -EINVAL;
+ }
+
+ if (port->qos.link_speed == SPEED_UNKNOWN)
+ return -ENOLINK;
+
+ if (taprio->cycle_time_extension) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "cycle time extension not supported");
Here it's ok.
quoted
+ return -EOPNOTSUPP;
}
Thanks for the detailed review!
--
cheers,
-roger
On 21/11/2023 11:23, Roger Quadros wrote:
On 21/11/2023 00:56, Vladimir Oltean wrote:
quoted
On Mon, Nov 20, 2023 at 04:01:42PM +0200, Roger Quadros wrote:
quoted
-static int am65_cpsw_configure_taprio(struct net_device *ndev,
- struct am65_cpsw_est *est_new)
+static void am65_cpsw_cp_taprio(struct tc_taprio_qopt_offload *from,
+ struct tc_taprio_qopt_offload *to)
+{
+ int i;
+
+ *to = *from;
+ for (i = 0; i < from->num_entries; i++)
+ to->entries[i] = from->entries[i];
+}
I think I mentioned this before: have you looked at taprio_offload_get()
and taprio_offload_put()?
I'm sorry that I missed this. I'll take a look.
Now I recollect. You mentioned this in a different series review
https://lore.kernel.org/all/20231011102536.r65xyzmh5kap2cf2@skbuf/
Since this patch is more trivial cleanups I will do the
taprio_offload_get/free() change to a separate patch.
quoted
quoted
+
+static int am65_cpsw_taprio_replace(struct net_device *ndev,
+ struct tc_taprio_qopt_offload *taprio)
{
struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
+ struct netlink_ext_ack *extack = taprio->mqprio.extack;
+ struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
struct am65_cpts *cpts = common->cpts;
int ret = 0, tact = TACT_PROG;
+ struct am65_cpsw_est *est_new;
- am65_cpsw_est_update_state(ndev);
+ if (!netif_running(ndev)) {
+ NL_SET_ERR_MSG_MOD(extack, "interface is down, link speed unknown\n");
The extack message doesn't need a \n.
OK.
quoted
quoted
+ return -ENETDOWN;
+ }
- if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
- am65_cpsw_stop_est(ndev);
- return ret;
+ if (common->pf_p0_rx_ptype_rrobin) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "p0-rx-ptype-rrobin flag conflicts with taprio qdisc\n");
Also here.
quoted
+ return -EINVAL;
+ }
+
+ if (port->qos.link_speed == SPEED_UNKNOWN)
+ return -ENOLINK;
+
+ if (taprio->cycle_time_extension) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "cycle time extension not supported");
Here it's ok.
quoted
+ return -EOPNOTSUPP;
}
Thanks for the detailed review!
--
cheers,
-roger