* Re: [PATCH net] ipvs: get sctphdr by sctphoff in sctp_csum_check
From: Simon Horman @ 2019-02-27 12:21 UTC (permalink / raw)
To: Xin Long
Cc: network dev, netfilter-devel, Marcelo Ricardo Leitner,
Neil Horman, pablo
In-Reply-To: <cd1c829cfd9ee916e57c51f6879947f0f88c1eb1.1551094063.git.lucien.xin@gmail.com>
On Mon, Feb 25, 2019 at 07:27:43PM +0800, Xin Long wrote:
> sctp_csum_check() is called by sctp_s/dnat_handler() where it calls
> skb_make_writable() to ensure sctphdr to be linearized.
>
> So there's no need to get sctphdr by calling skb_header_pointer()
> in sctp_csum_check().
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Simon Horman <horms@verge.net.au>
Pablo,
please consider applying this to nf-next.
> ---
> net/netfilter/ipvs/ip_vs_proto_sctp.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index b0cd7d0..0ecf241 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -183,7 +183,7 @@ static int
> sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
> {
> unsigned int sctphoff;
> - struct sctphdr *sh, _sctph;
> + struct sctphdr *sh;
> __le32 cmp, val;
>
> #ifdef CONFIG_IP_VS_IPV6
> @@ -193,10 +193,7 @@ sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
> #endif
> sctphoff = ip_hdrlen(skb);
>
> - sh = skb_header_pointer(skb, sctphoff, sizeof(_sctph), &_sctph);
> - if (sh == NULL)
> - return 0;
> -
> + sh = (struct sctphdr *)(skb->data + sctphoff);
> cmp = sh->checksum;
> val = sctp_compute_cksum(skb, sctphoff);
>
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH net-next 2/8] devlink: add PF and VF port flavours
From: Jiri Pirko @ 2019-02-27 12:23 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190226182436.23811-3-jakub.kicinski@netronome.com>
Tue, Feb 26, 2019 at 07:24:30PM CET, jakub.kicinski@netronome.com wrote:
>Current port flavours cover simple switches and DSA. Add PF
>and VF flavours to cover "switchdev" SR-IOV NICs.
>
>Example devlink user space output:
>
>$ devlink port
>pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
>pci/0000:82:00.0/10000: type eth netdev eth0 flavour pcie_pf pf 0
>pci/0000:82:00.0/10001: type eth netdev eth1 flavour pcie_vf pf 0 vf 0
>pci/0000:82:00.0/10002: type eth netdev eth2 flavour pcie_vf pf 0 vf 1
Wait a second, howcome pf and vfs have the same PCI address?
^ permalink raw reply
* Re: [PATCH net-next v2 7/8] net: switchdev: Replace port attr set SDO with a notification
From: Ido Schimmel @ 2019-02-27 12:28 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190227011427.16487-8-f.fainelli@gmail.com>
On Tue, Feb 26, 2019 at 05:14:26PM -0800, Florian Fainelli wrote:
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index af57c4a2b78a..b7988d49d708 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -67,12 +67,17 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
> .id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
> .u.brport_flags = mask,
> };
> + struct switchdev_notifier_port_attr_info info = {
> + .attr = &attr,
> + };
> int err;
>
> if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
> return 0;
>
> - err = switchdev_port_attr_set(p->dev, &attr);
> + /* We run from atomic context here */
> + err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev,
> + &info.info, NULL);
> if (err == -EOPNOTSUPP)
Florian, this needs to use notifier_to_errno() and check for any error.
With the ops, `-EOPNOTSUPP` was returned for devices that did not
implement `switchdev_ops`. Now they will simply not listen / reply to
the notification.
> return 0;
^ permalink raw reply
* Re: [PATCH net-next v2 1/8] switchdev: Add SWITCHDEV_PORT_ATTR_SET
From: Ido Schimmel @ 2019-02-27 12:30 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190227011427.16487-2-f.fainelli@gmail.com>
On Tue, Feb 26, 2019 at 05:14:20PM -0800, Florian Fainelli wrote:
> In preparation for allowing switchdev enabled drivers to veto specific
> attribute settings from within the context of the caller, introduce a
> new switchdev notifier type for port attributes.
>
> Suggested-by: Ido Schimmel <idosch@mellanox.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v2 4/8] mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_SET
From: Ido Schimmel @ 2019-02-27 12:30 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190227011427.16487-5-f.fainelli@gmail.com>
On Tue, Feb 26, 2019 at 05:14:23PM -0800, Florian Fainelli wrote:
> Following patches will change the way we communicate setting a port's
> attribute and use a notifier to perform those tasks.
>
> Prepare mlxsw to support receiving notifier events targeting
> SWITCHDEV_PORT_ATTR_SET and utilize the switchdev_handle_port_attr_set()
> to handle stacking of devices.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v2 8/8] net: Remove switchdev_ops
From: Ido Schimmel @ 2019-02-27 12:30 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190227011427.16487-9-f.fainelli@gmail.com>
On Tue, Feb 26, 2019 at 05:14:27PM -0800, Florian Fainelli wrote:
> Now that we have converted all possible callers to using a switchdev
> notifier for attributes we do not have a need for implementing
> switchdev_ops anymore, and this can be removed from all drivers the
> net_device structure.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 7/8] net: switchdev: Replace port attr set SDO with a notification
From: Ido Schimmel @ 2019-02-27 12:32 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <2e91b708-a092-3839-4979-2712145a9951@gmail.com>
On Mon, Feb 25, 2019 at 11:47:12AM -0800, Florian Fainelli wrote:
> On 2/25/19 1:49 AM, Ido Schimmel wrote:
> > On Sun, Feb 24, 2019 at 08:47:27AM -0800, Florian Fainelli wrote:
> >> Le 2/23/19 à 2:32 AM, Ido Schimmel a écrit :
> >>> On Fri, Feb 22, 2019 at 03:59:25PM -0800, Florian Fainelli wrote:
> >>>> - if (attr->flags & SWITCHDEV_F_NO_RECURSE)
> >>>> + if (attr & SWITCHDEV_F_DEFER)
> >>>> + rc = call_switchdev_blocking_notifiers(nt, dev,
> >>>> + &attr_info.info, NULL);
> >>>> + else
> >>>> + rc = call_switchdev_notifiers(nt, dev, &attr_info.info, NULL);
> >>>
> >>> I don't believe this is needed. You're calling this function from
> >>> switchdev_port_attr_set_now() which is always called from process
> >>> context. switchdev_port_attr_set() takes care of that. Similar to
> >>> switchdev_port_obj_add().
> >>
> >> Except for net/bridge/br_switchdev.c when we check the bridge port's
> >> flags support with PRE_BRIDGE_FLAGS. In that case we are executing from
> >> the caller (atomic) context and we can't defer otherwise that trumps the
> >> whole idea of being able to do a quick check and return that to the
> >> caller that we cannot support specific flags. How would you recommend
> >> approaching that?
> >
> > In this case you can invoke call_switchdev_notifiers() directly from
> > br_switchdev_set_port_flag(). Eventually switchdev_port_attr_set() will
> > be gone and bridge code will invoke the notifiers directly.
>
> That can be done, but it still requires the target driver (mlxsw,
> ocelot, dsa, etc.) to support attribute notification from blocking and
> non-blocking context. Are you fine with that?
Yes. Sorry for the latency. I was away yesterday. Reviewed your v2 and
only found one problem. Will run some tests now.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next 4/8] devlink: allow subports on devlink PCI ports
From: Jiri Pirko @ 2019-02-27 12:37 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev, parav, jgg
In-Reply-To: <20190226182436.23811-5-jakub.kicinski@netronome.com>
Tue, Feb 26, 2019 at 07:24:32PM CET, jakub.kicinski@netronome.com wrote:
>PCI endpoint corresponds to a PCI device, but such device
>can have one more more logical device ports associated with it.
>We need a way to distinguish those. Add a PCI subport in the
>dumps and print the info in phys_port_name appropriately.
>
>This is not equivalent to port splitting, there is no split
>group. It's just a way of representing multiple netdevs on
>a single PCI function.
>
>Note that the quality of being multiport pertains only to
>the PCI function itself. A PF having multiple netdevs does
>not mean that its VFs will also have multiple, or that VFs
>are associated with any particular port of a multiport VF.
>
We've been discussing the problem of subport (we call it "subfunction"
or "SF") for some time internally. Turned out, this is probably harder
task to model. Please prove me wrong.
The nature of VF makes it a logically separate entity. It has a separate
PCI address, it should therefore have a separate devlink instance.
You can pass it through to VM, then the same devlink instance should be
created inside the VM and disappear from the host.
SF (or subport) feels similar to that. Basically it is exactly the same
thing as VF, only does reside under PF PCI function.
That is why I think, for sake of consistency, it should have a separate
devlink entity as well. The problem is correct sysfs modelling and
devlink handle derived from that. Parav is working on a simple soft
bus for this purpose called "subbus". There is a RFC floating around on
Mellanox internal mailing list, looks like it is time to send it
upstream.
Then each PF driver which have SFs would register subbus devices
according to SFs/subports and they would be properly handled by bus
probe, devlink and devlink port and netdev instances created.
Ccing Parav and Jason.
^ permalink raw reply
* RE: [PATCH 4/8] qed: remove unnecessary memsets
From: Michal Kalderon @ 2019-02-27 12:39 UTC (permalink / raw)
To: Robert Eshleman
Cc: Tariq Toukan, David S. Miller, Ariel Elior, GR-everest-linux-l2,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <c15478ba2ff607f901c78bd56162509f1f023d52.1551246708.git.bobbyeshleman@gmail.com>
> From: Robert Eshleman <bobbyeshleman@gmail.com>
> Sent: Wednesday, February 27, 2019 8:10 AM
>
> This patch replaces unnecessary memset(,0) calls with simply assigning
> structs to zero.
>
> Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
> ---
> drivers/net/ethernet/qlogic/qed/qed_cxt.c | 4 +-
> drivers/net/ethernet/qlogic/qed/qed_hw.c | 3 +-
> drivers/net/ethernet/qlogic/qed/qed_mcp.c | 70 ++++++++---------------
> 3 files changed, 25 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
> b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
> index c2ad405b2f50..0452ef2fdf1d 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
> @@ -902,12 +902,10 @@ static int qed_cxt_src_t2_alloc(struct qed_hwfn
> *p_hwfn)
> struct qed_cxt_mngr *p_mngr = p_hwfn->p_cxt_mngr;
> u32 conn_num, total_size, ent_per_page, psz, i;
> struct qed_ilt_client_cfg *p_src;
> - struct qed_src_iids src_iids;
> + struct qed_src_iids src_iids = {0};
> struct qed_dma_mem *p_t2;
> int rc;
>
> - memset(&src_iids, 0, sizeof(src_iids));
> -
> /* if the SRC ILT client is inactive - there are no connection
> * requiring the searcer, leave.
> */
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_hw.c
> b/drivers/net/ethernet/qlogic/qed/qed_hw.c
> index 70504dcf4087..b8ca3a31409b 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_hw.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_hw.c
> @@ -831,7 +831,7 @@ int qed_dmae_sanity(struct qed_hwfn *p_hwfn,
> struct qed_ptt *p_ptt, const char *phase) {
> u32 size = PAGE_SIZE / 2, val;
> - struct qed_dmae_params params;
> + struct qed_dmae_params params = {0};
> int rc = 0;
> dma_addr_t p_phys;
> void *p_virt;
> @@ -864,7 +864,6 @@ int qed_dmae_sanity(struct qed_hwfn *p_hwfn,
> (u64)p_phys,
> p_virt, (u64)(p_phys + size), (u8 *)p_virt + size, size);
>
> - memset(¶ms, 0, sizeof(params));
> rc = qed_dmae_host2host(p_hwfn, p_ptt, p_phys, p_phys + size,
> size / 4 /* size_in_dwords */, ¶ms);
> if (rc) {
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
> b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
> index e7f18e34ff0d..e1b72fc819a9 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
> @@ -642,10 +642,9 @@ int qed_mcp_cmd(struct qed_hwfn *p_hwfn,
> u32 *o_mcp_resp,
> u32 *o_mcp_param)
> {
> - struct qed_mcp_mb_params mb_params;
> + struct qed_mcp_mb_params mb_params = {0};
> int rc;
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = cmd;
> mb_params.param = param;
>
> @@ -667,10 +666,9 @@ qed_mcp_nvm_wr_cmd(struct qed_hwfn *p_hwfn,
> u32 *o_mcp_resp,
> u32 *o_mcp_param, u32 i_txn_size, u32 *i_buf) {
> - struct qed_mcp_mb_params mb_params;
> + struct qed_mcp_mb_params mb_params = {0};
> int rc;
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = cmd;
> mb_params.param = param;
> mb_params.p_data_src = i_buf;
> @@ -695,11 +693,10 @@ int qed_mcp_nvm_rd_cmd(struct qed_hwfn
> *p_hwfn,
> u32 *o_mcp_resp,
> u32 *o_mcp_param, u32 *o_txn_size, u32 *o_buf) {
> - struct qed_mcp_mb_params mb_params;
> + struct qed_mcp_mb_params mb_params = {0};
> u8 raw_data[MCP_DRV_NVM_BUF_LEN];
> int rc;
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = cmd;
> mb_params.param = param;
> mb_params.p_data_dst = raw_data;
> @@ -821,13 +818,12 @@ __qed_mcp_load_req(struct qed_hwfn *p_hwfn,
> struct qed_load_req_in_params *p_in_params,
> struct qed_load_req_out_params *p_out_params) {
> - struct qed_mcp_mb_params mb_params;
> - struct load_req_stc load_req;
> + struct qed_mcp_mb_params mb_params = {0};
> + struct load_req_stc load_req = {0};
> struct load_rsp_stc load_rsp;
> u32 hsi_ver;
> int rc;
>
> - memset(&load_req, 0, sizeof(load_req));
> load_req.drv_ver_0 = p_in_params->drv_ver_0;
> load_req.drv_ver_1 = p_in_params->drv_ver_1;
> load_req.fw_ver = p_in_params->fw_ver; @@ -843,7 +839,6 @@
> __qed_mcp_load_req(struct qed_hwfn *p_hwfn,
> DRV_ID_MCP_HSI_VER_CURRENT :
> (p_in_params->hsi_ver << DRV_ID_MCP_HSI_VER_SHIFT);
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = DRV_MSG_CODE_LOAD_REQ;
> mb_params.param = PDA_COMP | hsi_ver | p_hwfn->cdev-
> >drv_type;
> mb_params.p_data_src = &load_req;
> @@ -959,12 +954,11 @@ int qed_mcp_load_req(struct qed_hwfn *p_hwfn,
> struct qed_ptt *p_ptt,
> struct qed_load_req_params *p_params) {
> - struct qed_load_req_out_params out_params;
> - struct qed_load_req_in_params in_params;
> + struct qed_load_req_out_params out_params = {0};
> + struct qed_load_req_in_params in_params = {0};
> u8 mfw_drv_role, mfw_force_cmd;
> int rc;
>
> - memset(&in_params, 0, sizeof(in_params));
> in_params.hsi_ver = QED_LOAD_REQ_HSI_VER_DEFAULT;
> in_params.drv_ver_0 = QED_VERSION;
> in_params.drv_ver_1 = qed_get_config_bitmap(); @@ -981,7 +975,6
> @@ int qed_mcp_load_req(struct qed_hwfn *p_hwfn,
> in_params.force_cmd = mfw_force_cmd;
> in_params.avoid_eng_reset = p_params->avoid_eng_reset;
>
> - memset(&out_params, 0, sizeof(out_params));
> rc = __qed_mcp_load_req(p_hwfn, p_ptt, &in_params,
> &out_params);
> if (rc)
> return rc;
> @@ -1072,7 +1065,7 @@ int qed_mcp_load_req(struct qed_hwfn *p_hwfn,
>
> int qed_mcp_unload_req(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
> {
> - struct qed_mcp_mb_params mb_params;
> + struct qed_mcp_mb_params mb_params = {0};
> u32 wol_param;
>
> switch (p_hwfn->cdev->wol_config) {
> @@ -1091,7 +1084,6 @@ int qed_mcp_unload_req(struct qed_hwfn
> *p_hwfn, struct qed_ptt *p_ptt)
> wol_param = DRV_MB_PARAM_UNLOAD_WOL_MCP;
> }
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = DRV_MSG_CODE_UNLOAD_REQ;
> mb_params.param = wol_param;
> mb_params.flags = QED_MB_FLAG_CAN_SLEEP |
> QED_MB_FLAG_AVOID_BLOCK; @@ -1101,17 +1093,15 @@ int
> qed_mcp_unload_req(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
>
> int qed_mcp_unload_done(struct qed_hwfn *p_hwfn, struct qed_ptt
> *p_ptt) {
> - struct qed_mcp_mb_params mb_params;
> - struct mcp_mac wol_mac;
> + struct qed_mcp_mb_params mb_params = {0};
> + struct mcp_mac wol_mac = {0};
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = DRV_MSG_CODE_UNLOAD_DONE;
>
> /* Set the primary MAC if WoL is enabled */
> if (p_hwfn->cdev->wol_config == QED_OV_WOL_ENABLED) {
> u8 *p_mac = p_hwfn->cdev->wol_mac;
>
> - memset(&wol_mac, 0, sizeof(wol_mac));
> wol_mac.mac_upper = p_mac[0] << 8 | p_mac[1];
> wol_mac.mac_lower = p_mac[2] << 24 | p_mac[3] << 16 |
> p_mac[4] << 8 | p_mac[5];
> @@ -1167,7 +1157,7 @@ int qed_mcp_ack_vf_flr(struct qed_hwfn
> *p_hwfn,
> u32 mfw_func_offsize = qed_rd(p_hwfn, p_ptt, addr);
> u32 func_addr = SECTION_ADDR(mfw_func_offsize,
> MCP_PF_ID(p_hwfn));
> - struct qed_mcp_mb_params mb_params;
> + struct qed_mcp_mb_params mb_params = {0};
> int rc;
> int i;
>
> @@ -1176,7 +1166,6 @@ int qed_mcp_ack_vf_flr(struct qed_hwfn
> *p_hwfn,
> "Acking VFs [%08x,...,%08x] - %08x\n",
> i * 32, (i + 1) * 32 - 1, vfs_to_ack[i]);
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = DRV_MSG_CODE_VF_DISABLED_DONE;
> mb_params.p_data_src = vfs_to_ack;
> mb_params.data_src_size = VF_MAX_STATIC / 8; @@ -1455,13
> +1444,12 @@ static void qed_mcp_handle_link_change(struct qed_hwfn
> *p_hwfn, int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct qed_ptt
> *p_ptt, bool b_up) {
> struct qed_mcp_link_params *params = &p_hwfn->mcp_info-
> >link_input;
> - struct qed_mcp_mb_params mb_params;
> - struct eth_phy_cfg phy_cfg;
> + struct qed_mcp_mb_params mb_params = {0};
> + struct eth_phy_cfg phy_cfg = {0};
> int rc = 0;
> u32 cmd;
>
> /* Set the shmem configuration according to params */
> - memset(&phy_cfg, 0, sizeof(phy_cfg));
> cmd = b_up ? DRV_MSG_CODE_INIT_PHY :
> DRV_MSG_CODE_LINK_RESET;
> if (!params->speed.autoneg)
> phy_cfg.speed = params->speed.forced_speed; @@ -1505,7
> +1493,6 @@ int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct
> qed_ptt *p_ptt, bool b_up)
> "Resetting link\n");
> }
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = cmd;
> mb_params.p_data_src = &phy_cfg;
> mb_params.data_src_size = sizeof(phy_cfg); @@ -1534,7 +1521,7
> @@ static void qed_mcp_send_protocol_stats(struct qed_hwfn *p_hwfn, {
> enum qed_mcp_protocol_type stats_type;
> union qed_mcp_protocol_stats stats;
> - struct qed_mcp_mb_params mb_params;
> + struct qed_mcp_mb_params mb_params = {0};
> u32 hsi_param;
>
> switch (type) {
> @@ -1561,7 +1548,6 @@ static void qed_mcp_send_protocol_stats(struct
> qed_hwfn *p_hwfn,
>
> qed_get_protocol_stats(p_hwfn->cdev, stats_type, &stats);
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = DRV_MSG_CODE_GET_STATS;
> mb_params.param = hsi_param;
> mb_params.p_data_src = &stats;
> @@ -2370,20 +2356,18 @@ qed_mcp_send_drv_version(struct qed_hwfn
> *p_hwfn,
> struct qed_ptt *p_ptt,
> struct qed_mcp_drv_version *p_ver)
> {
> - struct qed_mcp_mb_params mb_params;
> - struct drv_version_stc drv_version;
> + struct qed_mcp_mb_params mb_params = {0};
> + struct drv_version_stc drv_version = {0};
> __be32 val;
> u32 i;
> int rc;
>
> - memset(&drv_version, 0, sizeof(drv_version));
> drv_version.version = p_ver->version;
> for (i = 0; i < (MCP_DRV_VER_STR_SIZE - 4) / sizeof(u32); i++) {
> val = cpu_to_be32(*((u32 *)&p_ver->name[i *
> sizeof(u32)]));
> *(__be32 *)&drv_version.name[i * sizeof(u32)] = val;
> }
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = DRV_MSG_CODE_SET_VERSION;
> mb_params.p_data_src = &drv_version;
> mb_params.data_src_size = sizeof(drv_version); @@ -2536,11
> +2520,10 @@ int qed_mcp_ov_update_mtu(struct qed_hwfn *p_hwfn, int
> qed_mcp_ov_update_mac(struct qed_hwfn *p_hwfn,
> struct qed_ptt *p_ptt, u8 *mac)
> {
> - struct qed_mcp_mb_params mb_params;
> + struct qed_mcp_mb_params mb_params = {0};
> u32 mfw_mac[2];
> int rc;
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = DRV_MSG_CODE_SET_VMAC;
> mb_params.param = DRV_MSG_CODE_VMAC_TYPE_MAC <<
> DRV_MSG_CODE_VMAC_TYPE_SHIFT;
> @@ -3197,12 +3180,10 @@ qed_mcp_resc_allocation_msg(struct qed_hwfn
> *p_hwfn,
> struct qed_resc_alloc_in_params *p_in_params,
> struct qed_resc_alloc_out_params
> *p_out_params) {
> - struct qed_mcp_mb_params mb_params;
> - struct resource_info mfw_resc_info;
> + struct qed_mcp_mb_params mb_params = {0};
> + struct resource_info mfw_resc_info = {0};
> int rc;
>
> - memset(&mfw_resc_info, 0, sizeof(mfw_resc_info));
> -
> mfw_resc_info.res_id = qed_mcp_get_mfw_res_id(p_in_params-
> >res_id);
> if (mfw_resc_info.res_id == RESOURCE_NUM_INVALID) {
> DP_ERR(p_hwfn,
> @@ -3224,7 +3205,6 @@ qed_mcp_resc_allocation_msg(struct qed_hwfn
> *p_hwfn,
> return -EINVAL;
> }
>
> - memset(&mb_params, 0, sizeof(mb_params));
> mb_params.cmd = p_in_params->cmd;
> mb_params.param = QED_RESC_ALLOC_VERSION;
> mb_params.p_data_src = &mfw_resc_info; @@ -3277,15 +3257,13
> @@ qed_mcp_set_resc_max_val(struct qed_hwfn *p_hwfn,
> enum qed_resources res_id,
> u32 resc_max_val, u32 *p_mcp_resp)
> {
> - struct qed_resc_alloc_out_params out_params;
> - struct qed_resc_alloc_in_params in_params;
> + struct qed_resc_alloc_out_params out_params = {0};
> + struct qed_resc_alloc_in_params in_params = {0};
> int rc;
>
> - memset(&in_params, 0, sizeof(in_params));
> in_params.cmd = DRV_MSG_SET_RESOURCE_VALUE_MSG;
> in_params.res_id = res_id;
> in_params.resc_max_val = resc_max_val;
> - memset(&out_params, 0, sizeof(out_params));
> rc = qed_mcp_resc_allocation_msg(p_hwfn, p_ptt, &in_params,
> &out_params);
> if (rc)
> @@ -3302,14 +3280,12 @@ qed_mcp_get_resc_info(struct qed_hwfn
> *p_hwfn,
> enum qed_resources res_id,
> u32 *p_mcp_resp, u32 *p_resc_num, u32 *p_resc_start)
> {
> - struct qed_resc_alloc_out_params out_params;
> - struct qed_resc_alloc_in_params in_params;
> + struct qed_resc_alloc_out_params out_params = {0};
> + struct qed_resc_alloc_in_params in_params = {0};
> int rc;
>
> - memset(&in_params, 0, sizeof(in_params));
> in_params.cmd = DRV_MSG_GET_RESOURCE_ALLOC_MSG;
> in_params.res_id = res_id;
> - memset(&out_params, 0, sizeof(out_params));
> rc = qed_mcp_resc_allocation_msg(p_hwfn, p_ptt, &in_params,
> &out_params);
> if (rc)
> --
> 2.20.1
Thanks,
Acked-by: Michal Kalderon <michal.kalderon@marvell.com>
^ permalink raw reply
* Re: [PATCH net-next 1/3] dt-bindings: net: dsa: document additional Microchip KSZ9477 family switches
From: Andrew Lunn @ 2019-02-27 12:41 UTC (permalink / raw)
To: Tristram.Ha; +Cc: Florian Fainelli, Pavel Machek, UNGLinuxDriver, netdev
In-Reply-To: <1551224265-9304-2-git-send-email-Tristram.Ha@microchip.com>
On Tue, Feb 26, 2019 at 03:37:43PM -0800, Tristram.Ha@microchip.com wrote:
> + port@3 {
> + reg = <3>;
> + label = "lan4";
> + };
> + port@4 {
> + reg = <6>;
Hi Tristram
The number after the @ must be the same as the register. So this needs
to be port@6.
Andrew
> + label = "cpu";
> + ethernet = <ð0>;
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> + };
> };
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH net-next 2/8] devlink: add PF and VF port flavours
From: Jiri Pirko @ 2019-02-27 12:41 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190227122327.GA10571@nanopsycho>
Wed, Feb 27, 2019 at 01:23:27PM CET, jiri@resnulli.us wrote:
>Tue, Feb 26, 2019 at 07:24:30PM CET, jakub.kicinski@netronome.com wrote:
>>Current port flavours cover simple switches and DSA. Add PF
>>and VF flavours to cover "switchdev" SR-IOV NICs.
>>
>>Example devlink user space output:
>>
>>$ devlink port
>>pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
>>pci/0000:82:00.0/10000: type eth netdev eth0 flavour pcie_pf pf 0
>>pci/0000:82:00.0/10001: type eth netdev eth1 flavour pcie_vf pf 0 vf 0
>>pci/0000:82:00.0/10002: type eth netdev eth2 flavour pcie_vf pf 0 vf 1
>
>Wait a second, howcome pf and vfs have the same PCI address?
Oh, I think you have these as eswitch port representors. Confusing...
^ permalink raw reply
* [PATCH net-next v3 0/4] net/mlx5e: Make little improvement for mlx5e
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This serial patches are not bugfixes, and just little improvement for mlx5e.
v2->v3:
Patch 1 and 5 are merged to patch 4 now.
v1->v2
Patch 1: remove the duplicated error messages and stick to extack only
Patch 2: use the 'unknown' instead of empty string
Patch 5: is new patch
Tonghao Zhang (4):
net/mlx5e: Make the log friendly when decapsulation offload not
supported
net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()
net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr
net/mlx5e: Return -EOPNOTSUPP when attempting to offload an
unsupported action
.../net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 8 +++++---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 24 ++++++++++++++++------
2 files changed, 23 insertions(+), 9 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next v3 1/4] net/mlx5e: Make the log friendly when decapsulation offload not supported
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1551184063-40628-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
If we try to offload decapsulation actions to VFs hw, we get the log [1].
It's not friendly, because the kind of net device is null, and we don't
know what '0' means.
[1] "mlx5_core 0000:05:01.2 vf_0: decapsulation offload is not supported for net device (0)"
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
index bdcc5e7..6cbfbfa 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
@@ -84,7 +84,7 @@ static const char *mlx5e_netdev_kind(struct net_device *dev)
if (dev->rtnl_link_ops)
return dev->rtnl_link_ops->kind;
else
- return "";
+ return "unknown";
}
static int mlx5e_route_lookup_ipv6(struct mlx5e_priv *priv,
@@ -620,8 +620,10 @@ int mlx5e_tc_tun_parse(struct net_device *filter_dev,
headers_c, headers_v);
} else {
netdev_warn(priv->netdev,
- "decapsulation offload is not supported for %s net device (%d)\n",
- mlx5e_netdev_kind(filter_dev), tunnel_type);
+ "decapsulation offload is not supported for %s (kind: \"%s\")\n",
+ netdev_name(filter_dev),
+ mlx5e_netdev_kind(filter_dev));
+
return -EOPNOTSUPP;
}
return err;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 2/4] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1551184063-40628-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This patch is a little improvement. Simplify the parse_tc_fdb_actions().
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index b38986e..336a111 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2467,13 +2467,13 @@ static int parse_tc_vlan_action(struct mlx5e_priv *priv,
static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
struct flow_action *flow_action,
- struct mlx5e_tc_flow_parse_attr *parse_attr,
struct mlx5e_tc_flow *flow,
struct netlink_ext_ack *extack)
{
struct pedit_headers_action hdrs[2] = {};
struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
struct mlx5_esw_flow_attr *attr = flow->esw_attr;
+ struct mlx5e_tc_flow_parse_attr *parse_attr = attr->parse_attr;
struct mlx5e_rep_priv *rpriv = priv->ppriv;
const struct ip_tunnel_info *info = NULL;
const struct flow_action_entry *act;
@@ -2788,7 +2788,7 @@ static bool is_peer_flow_needed(struct mlx5e_tc_flow *flow)
if (err)
goto err_free;
- err = parse_tc_fdb_actions(priv, &rule->action, parse_attr, flow, extack);
+ err = parse_tc_fdb_actions(priv, &rule->action, flow, extack);
if (err)
goto err_free;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 4/4] net/mlx5e: Return -EOPNOTSUPP when attempting to offload an unsupported action
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1551184063-40628-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
* Now the encapsulation is not supported for mlx5 VFs. When we try to
offload that action, the -EINVAL is returned, but not -EOPNOTSUPP.
This patch changes the returned value and ignore to confuse user.
The command is shown as below [1].
* When max modify header action is zero, we return -EOPNOTSUPP
directly. In this way, we can ignore wrong message info (e.g.
"mlx5: parsed 0 pedit actions, can't do more"). This happens when
offloading pedit actions on mlx(cx4) VFs. The command is shown as below [2].
For example: (p2p1_0 is VF net device)
[1]
$ tc filter add dev p2p1_0 protocol ip parent ffff: prio 1 flower skip_sw \
src_mac e4:11:22:33:44:01 \
action tunnel_key set \
src_ip 1.1.1.100 \
dst_ip 1.1.1.200 \
dst_port 4789 id 100 \
action mirred egress redirect dev vxlan0
[2]
$ tc filter add dev p2p1_0 parent ffff: protocol ip prio 1 \
flower skip_sw dst_mac 00:10:56:fb:64:e8 \
dst_ip 1.1.1.100 src_ip 1.1.1.200 \
action pedit ex munge eth src set 00:10:56:b4:5d:20
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 56ac50d..3a02b22 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2035,7 +2035,7 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
struct netlink_ext_ack *extack)
{
u8 cmd = (act->id == FLOW_ACTION_MANGLE) ? 0 : 1;
- int err = -EOPNOTSUPP;
+ int max_actions, err = -EOPNOTSUPP;
u32 mask, val, offset;
u8 htype;
@@ -2047,6 +2047,17 @@ static int parse_tc_pedit_action(struct mlx5e_priv *priv,
goto out_err;
}
+ if (namespace == MLX5_FLOW_NAMESPACE_FDB) /* FDB offloading */
+ max_actions = MLX5_CAP_ESW_FLOWTABLE_FDB(priv->mdev, max_modify_header_actions);
+ else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
+ max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
+
+ if (!max_actions) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "don't support pedit actions, can't offload");
+ goto out_err;
+ }
+
mask = act->mangle.mask;
val = act->mangle.val;
offset = act->mangle.offset;
@@ -2294,7 +2305,8 @@ static int parse_tc_nic_actions(struct mlx5e_priv *priv,
}
break;
default:
- return -EINVAL;
+ NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
+ return -EOPNOTSUPP;
}
}
@@ -2616,7 +2628,8 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
break;
}
default:
- return -EINVAL;
+ NL_SET_ERR_MSG_MOD(extack, "The offload action is not supported");
+ return -EOPNOTSUPP;
}
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 3/4] net/mlx5e: Deletes unnecessary setting of esw_attr->parse_attr
From: xiangxia.m.yue @ 2019-02-26 12:27 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang
In-Reply-To: <1551184063-40628-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
This patch deletes unnecessary setting of the esw_attr->parse_attr
to parse_attr in parse_tc_fdb_actions() because it is already done
by the mlx5e_flow_esw_attr_init() function.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 336a111..56ac50d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2558,7 +2558,6 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
out_dev->ifindex;
parse_attr->tun_info[attr->out_count] = *info;
encap = false;
- attr->parse_attr = parse_attr;
attr->dests[attr->out_count].flags |=
MLX5_ESW_DEST_ENCAP;
attr->out_count++;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net v1] net/mlx5e: Correctly use the namespace type when allocating pedit action
From: xiangxia.m.yue @ 2019-02-26 12:28 UTC (permalink / raw)
To: saeedm, gerlitz.or, roid; +Cc: netdev, Tonghao Zhang, Pablo Neira Ayuso
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
The capacity of FDB offloading and NIC offloading table are
different, and when allocating the pedit actions, we should
use the correct namespace type.
Fixes: c500c86b0c75d ("net/mlx5e: support for two independent packet edit actions")
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 3a02b22..467ef9e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -2635,7 +2635,7 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
if (hdrs[TCA_PEDIT_KEY_EX_CMD_SET].pedits ||
hdrs[TCA_PEDIT_KEY_EX_CMD_ADD].pedits) {
- err = alloc_tc_pedit_action(priv, MLX5_FLOW_NAMESPACE_KERNEL,
+ err = alloc_tc_pedit_action(priv, MLX5_FLOW_NAMESPACE_FDB,
parse_attr, hdrs, extack);
if (err)
return err;
--
1.8.3.1
^ permalink raw reply related
* [PATCH][net-next] ethtool: Use explicit designated initializers for .cmd
From: Li RongQing @ 2019-02-27 12:47 UTC (permalink / raw)
To: netdev
Initialize the .cmd member by using a designated struct
initializer. This fixes warning of missing field initializers,
and makes code a little easier to read.
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/core/ethtool.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 47558ffae423..d4918ffddda8 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1717,7 +1717,7 @@ static noinline_for_stack int ethtool_set_channels(struct net_device *dev,
static int ethtool_get_pauseparam(struct net_device *dev, void __user *useraddr)
{
- struct ethtool_pauseparam pauseparam = { ETHTOOL_GPAUSEPARAM };
+ struct ethtool_pauseparam pauseparam = { .cmd = ETHTOOL_GPAUSEPARAM };
if (!dev->ethtool_ops->get_pauseparam)
return -EOPNOTSUPP;
@@ -2503,7 +2503,7 @@ static int set_phy_tunable(struct net_device *dev, void __user *useraddr)
static int ethtool_get_fecparam(struct net_device *dev, void __user *useraddr)
{
- struct ethtool_fecparam fecparam = { ETHTOOL_GFECPARAM };
+ struct ethtool_fecparam fecparam = { .cmd = ETHTOOL_GFECPARAM };
int rc;
if (!dev->ethtool_ops->get_fecparam)
--
2.16.2
^ permalink raw reply related
* Re: [PATCH net] sctp: get sctphdr by offset in sctp_compute_cksum
From: Xin Long @ 2019-02-27 12:53 UTC (permalink / raw)
To: Neil Horman; +Cc: LKML, network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <20190226122902.GA13064@hmswarspite.think-freely.org>
On Tue, Feb 26, 2019 at 8:29 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Tue, Feb 26, 2019 at 12:15:54AM +0800, Xin Long wrote:
> > On Mon, Feb 25, 2019 at 10:08 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > On Mon, Feb 25, 2019 at 09:20:44PM +0800, Xin Long wrote:
> > > > On Mon, Feb 25, 2019 at 8:47 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > >
> > > > > On Mon, Feb 25, 2019 at 07:25:37PM +0800, Xin Long wrote:
> > > > > > sctp_hdr(skb) only works when skb->transport_header is set properly.
> > > > > >
> > > > > > But in the path of nf_conntrack_in: sctp_packet() -> sctp_error()
> > > > > >
> > > > > > skb->transport_header is not guaranteed to be right value for sctp.
> > > > > > It will cause to fail to check the checksum for sctp packets.
> > > > > >
> > > > > > So fix it by using offset, which is always right in all places.
> > > > > >
> > > > > > Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code")
> > > > > > Reported-by: Li Shuang <shuali@redhat.com>
> > > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > > ---
> > > > > > include/net/sctp/checksum.h | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> > > > > > index 32ee65a..1c6e6c0 100644
> > > > > > --- a/include/net/sctp/checksum.h
> > > > > > +++ b/include/net/sctp/checksum.h
> > > > > > @@ -61,7 +61,7 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
> > > > > > static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
> > > > > > unsigned int offset)
> > > > > > {
> > > > > > - struct sctphdr *sh = sctp_hdr(skb);
> > > > > > + struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
> > > > > > const struct skb_checksum_ops ops = {
> > > > > > .update = sctp_csum_update,
> > > > > > .combine = sctp_csum_combine,
> > > > > > --
> > > > > > 2.1.0
> > > > > >
> > > > > >
> > > > > Shouldn't you use skb_set_transport_header and skb_transport_header here?
> > > > you mean:
> > > > skb_set_transport_header(skb, offset);
> > > > sh = sctp_hdr(skb);
> > > > ?
> > > >
> > > > There's no place counting on here to set transport_header.
> > > > It will be a kinda redundant job, yet skb is 'const'.
> > > >
> > > I'm not sure what you mean by "theres no place counting here". We have the
> > > transport header offset, and you're doing the exact same computation that that
> > > function does. It seems like we should use it in case the underlying
> > > implementation changes.
> > 1. skb_set_transport_header() and sctp_hdr() are like:
> > skb->transport_header = skb->data - skb->head;
> > skb->transport_header += offset
> > sh = skb->head + skb->transport_header;
> >
> > 2. in this patch:
> > sh = (struct sctphdr *)(skb->data + offset); only
> >
> > I think the 2nd one is better.
> >
> > I feel it's weird to set transport_header here if it's only for
> > sctp_hdr(skb) in here.
> >
> > As for "underlying implementation changes", I don't know exactly the case
> > but there are quite a few places doing things like:
> > *hdr = (struct *hdr *)(skb->data + hdroff);
> >
> > I'd think it's safe. no?
> >
> Safe, yes, it just doesn't seem right. I know you've pointed out several places
> below that rapidly compute transport offsets in a one-off fashion, but at this
> same time, the other primary transports (tcp and udp), all seems to use the
> transport header to do their work (linearizing as necessecary, which sctp also
> does in sctp_rcv, at least in most cases).
> > >
> > > I understand what you are saying regarding the use of a const variable there,
> > > but perhaps thats an argument for removing the const storage classifier. Better
> > > still, it would be good to figure out why all paths to this function don't
> > > already set the transport header offset to begin with (addressing your redundant
> > > comment)
> > The issue was reported when going to nf_conntrack by br_netfilter's
> > bridge-nf-call-iptables.
> > As you can see on nf_conntrack_in() path, even iphdr is got by:
> > iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
> > It's impossible to set skb->transport_header when we're not sure iphdr
> > in linearized memory.
> >
> But if the skb isn't linearized, computing the transport header manually isn't
> going to help you anyway. You can see that in skb_header_pointer. If the
> offset they are trying to get to is outside the bounds of the length of the skb
> (i.e. the fragmented case), it calls skb_copy_bits to linearize the needed
> segment. It seems we should be doing something simmilar. In most cases we are
> already linearized from sctp_rcv (possibly all, I need to think about that). All
> I'm really saying is that by using the skb apis we insulate ourselves from
> potential changes in how skbs might work in the future. I'm not strictly bound
> to setting the transport header, but we should definately be getting the
> transport header via the skb utility functions wherever possible.
Okay, I will change to fix it with the below patch if you agree.
I've confirmed this won't affect netfilter.
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c
b/net/netfilter/nf_conntrack_proto_sctp.c
index d53e3e7..6b53cd2 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -343,7 +343,9 @@ static bool sctp_error(struct sk_buff *skb,
logmsg = "nf_ct_sctp: failed to read header ";
goto out_invalid;
}
- sh = (const struct sctphdr *)(skb->data + dataoff);
+ /* sctp_compute_cksum() depends on correct transport header */
+ skb_set_transport_header(skb, dataoff);
+ sh = sctp_hdr(skb);
^ permalink raw reply related
* Re: [PATCH net-next 6/8] devlink: introduce port's peer netdevs
From: Jiri Pirko @ 2019-02-27 13:08 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190226182436.23811-7-jakub.kicinski@netronome.com>
Tue, Feb 26, 2019 at 07:24:34PM CET, jakub.kicinski@netronome.com wrote:
>Devlink ports represent ports of a switch device (or SR-IOV
>NIC which has an embedded switch). In case of SR-IOV when
>PCIe PFs are exposed the PFs which are directly connected
>to the local machine may also spawn PF netdev (much like
>VFs have a port/"repr" and an actual VF netdev).
>
>Allow devlink to expose such linking. There is currently no
>way to find out which netdev corresponds to which PF.
>
>Example:
>
>$ devlink port
>pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
>pci/0000:82:00.0/10000: type eth netdev eth1 flavour pci_pf pf 0 peer_netdev enp130s0
>pci/0000:82:00.0/10001: type eth netdev eth0 flavour pci_vf pf 0 vf 0
>pci/0000:82:00.0/10002: type eth netdev eth2 flavour pci_vf pf 0 vf 1
Peer as the other side of a "virtual cable". For PF, that is probably
sufficient. But I think what a "peer of devlink port" should be "a
devlink port".
Not sure about VF.
Consider a simple problem of setting up a VF mac address. In legacy, you
do it like this:
$ ip link set eth2 vf 1 mac 00:52:44:11:22:33
However, in new model, you so far cannot do that.
What I was thinking about was some "dummy peer" which would be on the
host. Not sure if only as a "dummy peer devlink port" or even as some
sort of "dummy netdev".
One way or another, it would provide the user some info about which VF
representor is connected to which VF in VM (mac mapping).
^ permalink raw reply
* Re: [PATCH net-next v2 8/8] net: Remove switchdev_ops
From: Ido Schimmel @ 2019-02-27 13:13 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, David S. Miller, open list,
open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE,
Jiri Pirko, andrew@lunn.ch, vivien.didelot@gmail.com
In-Reply-To: <20190227011427.16487-9-f.fainelli@gmail.com>
On Tue, Feb 26, 2019 at 05:14:27PM -0800, Florian Fainelli wrote:
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index b00f6f74f91a..995426ea9a43 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -3660,7 +3660,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
> }
> mlxsw_sp_port->default_vlan = mlxsw_sp_port_vlan;
>
> - mlxsw_sp_port_switchdev_init(mlxsw_sp_port);
> mlxsw_sp->ports[local_port] = mlxsw_sp_port;
> err = register_netdev(dev);
> if (err) {
> @@ -3677,7 +3676,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
>
> err_register_netdev:
> mlxsw_sp->ports[local_port] = NULL;
> - mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
> mlxsw_sp_port_vlan_destroy(mlxsw_sp_port_vlan);
> err_port_vlan_create:
> err_port_pvid_set:
> @@ -3720,7 +3718,6 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
> mlxsw_core_port_clear(mlxsw_sp->core, local_port, mlxsw_sp);
> unregister_netdev(mlxsw_sp_port->dev); /* This calls ndo_stop */
> mlxsw_sp->ports[local_port] = NULL;
> - mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
> mlxsw_sp_port_vlan_flush(mlxsw_sp_port, true);
> mlxsw_sp_port_nve_fini(mlxsw_sp_port);
> mlxsw_sp_tc_qdisc_fini(mlxsw_sp_port);
> @@ -4441,12 +4438,6 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
> goto err_span_init;
> }
>
> - err = mlxsw_sp_switchdev_init(mlxsw_sp);
I missed that and got a trace as soon as I tried to enslave a port. You
should only remove mlxsw_sp_port_switchdev_init() and not
mlxsw_sp_switchdev_init()
> - if (err) {
> - dev_err(mlxsw_sp->bus_info->dev, "Failed to initialize switchdev\n");
> - goto err_switchdev_init;
> - }
> -
> err = mlxsw_sp_counter_pool_init(mlxsw_sp);
> if (err) {
> dev_err(mlxsw_sp->bus_info->dev, "Failed to init counter pool\n");
> @@ -4517,8 +4508,6 @@ static int mlxsw_sp_init(struct mlxsw_core *mlxsw_core,
> err_afa_init:
> mlxsw_sp_counter_pool_fini(mlxsw_sp);
> err_counter_pool_init:
> - mlxsw_sp_switchdev_fini(mlxsw_sp);
> -err_switchdev_init:
> mlxsw_sp_span_fini(mlxsw_sp);
> err_span_init:
> mlxsw_sp_lag_fini(mlxsw_sp);
> @@ -4585,7 +4574,6 @@ static void mlxsw_sp_fini(struct mlxsw_core *mlxsw_core)
> mlxsw_sp_nve_fini(mlxsw_sp);
> mlxsw_sp_afa_fini(mlxsw_sp);
> mlxsw_sp_counter_pool_fini(mlxsw_sp);
> - mlxsw_sp_switchdev_fini(mlxsw_sp);
> mlxsw_sp_span_fini(mlxsw_sp);
> mlxsw_sp_lag_fini(mlxsw_sp);
> mlxsw_sp_buffers_fini(mlxsw_sp);
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> index a61c1130d9e3..da6278b0caa4 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> @@ -407,8 +407,6 @@ extern const struct mlxsw_sp_sb_vals mlxsw_sp2_sb_vals;
> /* spectrum_switchdev.c */
> int mlxsw_sp_switchdev_init(struct mlxsw_sp *mlxsw_sp);
> void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp);
> -void mlxsw_sp_port_switchdev_init(struct mlxsw_sp_port *mlxsw_sp_port);
> -void mlxsw_sp_port_switchdev_fini(struct mlxsw_sp_port *mlxsw_sp_port);
> int mlxsw_sp_rif_fdb_op(struct mlxsw_sp *mlxsw_sp, const char *mac, u16 fid,
> bool adding);
> void
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> index c1aedfea3a31..f6ce386c3036 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
> @@ -1938,10 +1938,6 @@ static struct mlxsw_sp_port *mlxsw_sp_lag_rep_port(struct mlxsw_sp *mlxsw_sp,
> return NULL;
> }
>
> -static const struct switchdev_ops mlxsw_sp_port_switchdev_ops = {
> - .switchdev_port_attr_set = mlxsw_sp_port_attr_set,
> -};
> -
> static int
> mlxsw_sp_bridge_8021q_port_join(struct mlxsw_sp_bridge_device *bridge_device,
> struct mlxsw_sp_bridge_port *bridge_port,
> @@ -3545,11 +3541,3 @@ void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp)
> kfree(mlxsw_sp->bridge);
> }
>
> -void mlxsw_sp_port_switchdev_init(struct mlxsw_sp_port *mlxsw_sp_port)
> -{
> - mlxsw_sp_port->dev->switchdev_ops = &mlxsw_sp_port_switchdev_ops;
> -}
> -
> -void mlxsw_sp_port_switchdev_fini(struct mlxsw_sp_port *mlxsw_sp_port)
> -{
> -}
^ permalink raw reply
* Re: [PATCH net-next 8/8] devlink: fix kdoc
From: Jiri Pirko @ 2019-02-27 13:13 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190226182436.23811-9-jakub.kicinski@netronome.com>
Tue, Feb 26, 2019 at 07:24:36PM CET, jakub.kicinski@netronome.com wrote:
>devlink suffers from a few kdoc warnings:
>
>net/core/devlink.c:5292: warning: Function parameter or member 'dev' not described in 'devlink_register'
>net/core/devlink.c:5351: warning: Function parameter or member 'port_index' not described in 'devlink_port_register'
>net/core/devlink.c:5753: warning: Function parameter or member 'parent_resource_id' not described in 'devlink_resource_register'
>net/core/devlink.c:5753: warning: Function parameter or member 'size_params' not described in 'devlink_resource_register'
>net/core/devlink.c:5753: warning: Excess function parameter 'top_hierarchy' description in 'devlink_resource_register'
>net/core/devlink.c:5753: warning: Excess function parameter 'reload_required' description in 'devlink_resource_register'
>net/core/devlink.c:5753: warning: Excess function parameter 'parent_reosurce_id' description in 'devlink_resource_register'
>net/core/devlink.c:6451: warning: Function parameter or member 'region' not described in 'devlink_region_snapshot_create'
>net/core/devlink.c:6451: warning: Excess function parameter 'devlink_region' description in 'devlink_region_snapshot_create'
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: dsa: microchip: add KSZ9893 switch support
From: Andrew Lunn @ 2019-02-27 13:14 UTC (permalink / raw)
To: Tristram.Ha; +Cc: Florian Fainelli, Pavel Machek, UNGLinuxDriver, netdev
In-Reply-To: <1551224265-9304-3-git-send-email-Tristram.Ha@microchip.com>
> static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg)
> @@ -353,7 +363,7 @@ static int ksz9477_phy_read16(struct dsa_switch *ds, int addr, int reg)
> val = 0x796d;
> break;
> case MII_PHYSID1:
> - val = 0x0022;
> + val = KSZ9477_ID_HI;
> break;
> case MII_PHYSID2:
> val = 0x1631;
Hi Tristram
This seems like an unrelated change, so should be in a separate
patch. And if you change PHYID1, it would also make sense to change
PHYID2 to a #define.
Please split the tagging changes into a patch of there own.
Andrew
^ permalink raw reply
* Re: [PATCH v4 perf,bpf 14/15] perf: introduce side band thread
From: Jiri Olsa @ 2019-02-27 13:21 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190226002019.3748539-15-songliubraving@fb.com>
On Mon, Feb 25, 2019 at 04:20:18PM -0800, Song Liu wrote:
SNIP
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 8c902276d4b4..61b87c8111e6 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -19,6 +19,7 @@
> #include "debug.h"
> #include "units.h"
> #include "asm/bug.h"
> +#include "bpf-event.h"
> #include <signal.h>
> #include <unistd.h>
>
> @@ -1841,3 +1842,102 @@ struct perf_evsel *perf_evlist__reset_weak_group(struct perf_evlist *evsel_list,
> }
> return leader;
> }
> +
> +static struct perf_evlist *sb_evlist;
> +pthread_t poll_thread;
so some of the things are static and some like poll_args
you alloced on the stack.. I dont like this interface,
could we come up with something generic? perhaps
encapsulated in perf_evlist, like:
struct perf_evlist {
...
struct {
pthread_t th;
int state;
} thread;
};
typedef int (perf_evlist__thread_cb_t)(perf_evlist, union perf_event *event,....)
perf_evlist__start_thread(perf_evlist, perf_evlist__thread_cb_t cb);
perf_evlist__stop_thread(perf_evlist);
jirka
^ permalink raw reply
* Re: [PATCH v4 perf,bpf 15/15] perf, bpf: save information about short living bpf programs
From: Jiri Olsa @ 2019-02-27 13:21 UTC (permalink / raw)
To: Song Liu
Cc: netdev, linux-kernel, ast, daniel, kernel-team, peterz, acme,
jolsa, namhyung
In-Reply-To: <20190226002019.3748539-16-songliubraving@fb.com>
On Mon, Feb 25, 2019 at 04:20:19PM -0800, Song Liu wrote:
SNIP
> + btf_id = info_linear->info.btf_id;
> +
> + info_node = malloc(sizeof(struct bpf_prog_info_node));
> + if (info_node) {
> + info_node->info_linear = info_linear;
> + perf_env__insert_bpf_prog_info(env, info_node);
> + } else
> + free(info_linear);
> +
> + if (btf_id == 0)
> + goto out;
> +
> + if (btf__get_from_id(btf_id, &btf)) {
> + pr_debug("%s: failed to get BTF of id %u, aborting\n",
> + __func__, btf_id);
> + goto out;
> + }
> + perf_env__fetch_btf(env, btf_id, btf);
so is this the main reason we are doing this? getting the btf
data for bpf prog ids and store them?
please describe the whole bpf events/features data flow in
changelog as I asked in previous email
thanks,
jirka
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox