Message ID | 20231218082758.247831-1-sumang@marvell.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-3194-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:24d3:b0:fb:cd0c:d3e with SMTP id r19csp1110209dyi; Mon, 18 Dec 2023 00:57:50 -0800 (PST) X-Google-Smtp-Source: AGHT+IHudrJsBIRnaGHEK4wmui9Imm5D1bboFBeXUgVjPKnoDS13hYwa5j676SQPEAhy9X1nto2M X-Received: by 2002:ad4:5ecc:0:b0:67f:4856:87be with SMTP id jm12-20020ad45ecc000000b0067f485687bemr1142199qvb.8.1702889870742; Mon, 18 Dec 2023 00:57:50 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1702889870; cv=none; d=google.com; s=arc-20160816; b=iubhJek9D4lp1xTkuLgX3dfPJP12j0C2xPh7VHo+n5qpIggvY6YzyNbEAw6JpVnjLx H8nv4RTZNobE/a2lUZWcZWjHx23oZsLk+7LvnOjU1EtwQGhf4tbf681oi7TO4LlFPvuy X4IBMoqkmTukFygt6MfzHHD29rxTPoJKY+DoOPhWzsIBHN8AWmTn9Y9QwJndp9Z3chjc Gih7y93XgpJ5jAgChoeXmafNd/8qKZkxtY4hnXbJ5JnQL/Hejq89J766SXOuKlwLR3Ss 9WOn7t69q5rAptcFZIC28/y5+j6HCYPS1rDEfNgXUJVIIY2kuxsVAdGdAT/9Y2ioRuRN Buug== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=P9HCpRvocxgDIAlncH65HWa5yp44CPybVsfOANN0Cxc=; fh=pK51JAizWKp9jwq8870a7Zs4DJFrvE+hT0N4i+cGlR0=; b=Y8pw3QXShBD3QOyvVk4drc4nbvjiSBlr6qj1ZRiPJSBXZezzcn7y7T7ACEK5F8jTEd BNSqK3X8V2DRWqatL4/ceC39Iz1pRozpuEpekWg8GHmyS3nif2y79XrGz+FT1KIZmOLY ywxeJzxf5N6lUcYfNpb34MV4rxSP0ow64INSK82a/rkH/XAEa/mOy3tkWbvm8Doa5JBX KYMIM/HcQRKUSFe4SzAxid6hceiZhi+wM/s6PrRL9b6Po8ztrw1BZrB0trK8klI6mQZm ElkRV3JsTeoq4OVm3hZ+fwrZm7xu1YjB6TEDjugkPaS/cA2cLmp39MQsapnX8RP0Q9hg hSHw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@marvell.com header.s=pfpt0220 header.b=HqwLgHin; spf=pass (google.com: domain of linux-kernel+bounces-3194-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-3194-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=REJECT dis=NONE) header.from=marvell.com Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [2604:1380:45d1:ec00::1]) by mx.google.com with ESMTPS id x3-20020a0ce243000000b0067f2558d75fsi5500101qvl.558.2023.12.18.00.57.50 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Dec 2023 00:57:50 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-3194-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1; Authentication-Results: mx.google.com; dkim=pass header.i=@marvell.com header.s=pfpt0220 header.b=HqwLgHin; spf=pass (google.com: domain of linux-kernel+bounces-3194-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-3194-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=REJECT dis=NONE) header.from=marvell.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 87A611C212AD for <ouuuleilei@gmail.com>; Mon, 18 Dec 2023 08:57:50 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 486DA11713; Mon, 18 Dec 2023 08:57:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=marvell.com header.i=@marvell.com header.b="HqwLgHin" X-Original-To: linux-kernel@vger.kernel.org Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2A15B111A1; Mon, 18 Dec 2023 08:57:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=marvell.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=marvell.com Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.17.1.24/8.17.1.24) with ESMTP id 3BI3LPCC020756; Mon, 18 Dec 2023 00:57:16 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h= from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding:content-type; s=pfpt0220; bh=P9HCpRvo cxgDIAlncH65HWa5yp44CPybVsfOANN0Cxc=; b=HqwLgHinFwzACcXxbBHL3emG IFK3RVCaOPHASwKkZmDTQYufZpAs9cMnbp8TwGlskVkWCJqA92t5xVSAzxBX+CJf CgEOE4qyYSDvN38iqCqsnkrpNjeuITIC0+MHL7g5yLZs3UG7f0MmMY50du2OCW7k ErMyp67gQ/BceCfuoq9qeEb+Hz8UvRYO4jlfEzFkYaN/2325WPJr5xCNDqnMry1a z4OFXrwDnSJmZIwoOSDSHd+ADaSO0gIxIU/g+Axow+4XY+eXYbrxoudS3Y6TQyEI 5jfbkH6B9EBMrkKHdWfVCrSb+gZsDk7aWgNIFjSsx103HxY4UtdRCVOA1054vA== Received: from dc5-exch02.marvell.com ([199.233.59.182]) by mx0a-0016f401.pphosted.com (PPS) with ESMTPS id 3v2e3v8y3c-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Mon, 18 Dec 2023 00:57:16 -0800 (PST) Received: from DC5-EXCH02.marvell.com (10.69.176.39) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server (TLS) id 15.0.1497.48; Mon, 18 Dec 2023 00:57:15 -0800 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH02.marvell.com (10.69.176.39) with Microsoft SMTP Server id 15.0.1497.48 via Frontend Transport; Mon, 18 Dec 2023 00:57:15 -0800 Received: from localhost.localdomain (unknown [10.28.36.166]) by maili.marvell.com (Postfix) with ESMTP id 48E643F7970; Mon, 18 Dec 2023 00:28:01 -0800 (PST) From: Suman Ghosh <sumang@marvell.com> To: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>, <sgoutham@marvell.com>, <sbhatta@marvell.com>, <jerinj@marvell.com>, <gakula@marvell.com>, <hkelam@marvell.com>, <lcherian@marvell.com>, <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org> CC: Suman Ghosh <sumang@marvell.com> Subject: [net PATCH] octeontx2-af: Fix marking couple of structure as __packed Date: Mon, 18 Dec 2023 13:57:58 +0530 Message-ID: <20231218082758.247831-1-sumang@marvell.com> X-Mailer: git-send-email 2.25.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Proofpoint-ORIG-GUID: eE_W25v32nyC96SHiaYXWkHuFXnEU8xR X-Proofpoint-GUID: eE_W25v32nyC96SHiaYXWkHuFXnEU8xR X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-12-09_02,2023-12-07_01,2023-05-22_02 X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1785609449121032249 X-GMAIL-MSGID: 1785609449121032249 |
Series |
[net] octeontx2-af: Fix marking couple of structure as __packed
|
|
Commit Message
Suman Ghosh
Dec. 18, 2023, 8:27 a.m. UTC
Couple of structures was not marked as __packed which may have some
performance implication. This patch fixes the same and mark them as
__packed.
Fixes: 42006910b5ea ("octeontx2-af: cleanup KPU config data")
Signed-off-by: Suman Ghosh <sumang@marvell.com>
---
drivers/net/ethernet/marvell/octeontx2/af/npc.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On 12/18/2023 12:27 AM, Suman Ghosh wrote: > Couple of structures was not marked as __packed which may have some > performance implication. This patch fixes the same and mark them as > __packed. Not sure I follow why lack of __packed would have performance implications? I get that __packed is important to ensure layout is correct or to ensure the whole structure has the right size rather than unexpected gaps. I'd guess maybe because the structures size would include padding without __packed, leading to a lot of gaps when combining several structures together... I did test on my system with pahole, and even without __packed, I don't get any gaps in the npc_lt_def_cfg structure: > struct npc_lt_def_cfg { > struct npc_lt_def rx_ol2; /* 0 3 */ > struct npc_lt_def rx_oip4; /* 3 3 */ > struct npc_lt_def rx_iip4; /* 6 3 */ > struct npc_lt_def rx_oip6; /* 9 3 */ > struct npc_lt_def rx_iip6; /* 12 3 */ > struct npc_lt_def rx_otcp; /* 15 3 */ > struct npc_lt_def rx_itcp; /* 18 3 */ > struct npc_lt_def rx_oudp; /* 21 3 */ > struct npc_lt_def rx_iudp; /* 24 3 */ > struct npc_lt_def rx_osctp; /* 27 3 */ > struct npc_lt_def rx_isctp; /* 30 3 */ > struct npc_lt_def_ipsec rx_ipsec[2]; /* 33 10 */ > struct npc_lt_def pck_ol2; /* 43 3 */ > struct npc_lt_def pck_oip4; /* 46 3 */ > struct npc_lt_def pck_oip6; /* 49 3 */ > struct npc_lt_def pck_iip4; /* 52 3 */ > struct npc_lt_def_apad rx_apad0; /* 55 4 */ > struct npc_lt_def_apad rx_apad1; /* 59 4 */ > struct npc_lt_def_color ovlan; /* 63 5 */ > /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */ > struct npc_lt_def_color ivlan; /* 68 5 */ > struct npc_lt_def_color rx_gen0_color; /* 73 5 */ > struct npc_lt_def_color rx_gen1_color; /* 78 5 */ > struct npc_lt_def_et rx_et[2]; /* 83 10 */ > > /* size: 93, cachelines: 2, members: 23 */ > /* last cacheline: 29 bytes */ > }; However that may not be true across all compilers etc. Also all the other structures are __packed. Makes sense. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > > Fixes: 42006910b5ea ("octeontx2-af: cleanup KPU config data") > Signed-off-by: Suman Ghosh <sumang@marvell.com> > --- > drivers/net/ethernet/marvell/octeontx2/af/npc.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/npc.h b/drivers/net/ethernet/marvell/octeontx2/af/npc.h > index ab3e39eef2eb..8c0732c9a7ee 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/af/npc.h > +++ b/drivers/net/ethernet/marvell/octeontx2/af/npc.h > @@ -528,7 +528,7 @@ struct npc_lt_def { > u8 ltype_mask; > u8 ltype_match; > u8 lid; > -}; > +} __packed; > > struct npc_lt_def_ipsec { > u8 ltype_mask; > @@ -536,7 +536,7 @@ struct npc_lt_def_ipsec { > u8 lid; > u8 spi_offset; > u8 spi_nz; > -}; > +} __packed; > > struct npc_lt_def_apad { > u8 ltype_mask;
>Not sure I follow why lack of __packed would have performance >implications? I get that __packed is important to ensure layout is >correct or to ensure the whole structure has the right size rather than >unexpected gaps. I'd guess maybe because the structures size would >include padding without __packed, leading to a lot of gaps when >combining several structures together... > >I did test on my system with pahole, and even without __packed, I don't >get any gaps in the npc_lt_def_cfg structure: > > >> struct npc_lt_def_cfg { >> struct npc_lt_def rx_ol2; /* 0 >3 */ >> struct npc_lt_def rx_oip4; /* 3 >3 */ >> struct npc_lt_def rx_iip4; /* 6 >3 */ >> struct npc_lt_def rx_oip6; /* 9 >3 */ >> struct npc_lt_def rx_iip6; /* 12 >3 */ >> struct npc_lt_def rx_otcp; /* 15 >3 */ >> struct npc_lt_def rx_itcp; /* 18 >3 */ >> struct npc_lt_def rx_oudp; /* 21 >3 */ >> struct npc_lt_def rx_iudp; /* 24 >3 */ >> struct npc_lt_def rx_osctp; /* 27 >3 */ >> struct npc_lt_def rx_isctp; /* 30 >3 */ >> struct npc_lt_def_ipsec rx_ipsec[2]; /* 33 >10 */ >> struct npc_lt_def pck_ol2; /* 43 >3 */ >> struct npc_lt_def pck_oip4; /* 46 >3 */ >> struct npc_lt_def pck_oip6; /* 49 >3 */ >> struct npc_lt_def pck_iip4; /* 52 >3 */ >> struct npc_lt_def_apad rx_apad0; /* 55 >4 */ >> struct npc_lt_def_apad rx_apad1; /* 59 >4 */ >> struct npc_lt_def_color ovlan; /* 63 >5 */ >> /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */ >> struct npc_lt_def_color ivlan; /* 68 >5 */ >> struct npc_lt_def_color rx_gen0_color; /* 73 >5 */ >> struct npc_lt_def_color rx_gen1_color; /* 78 >5 */ >> struct npc_lt_def_et rx_et[2]; /* 83 >10 */ >> >> /* size: 93, cachelines: 2, members: 23 */ >> /* last cacheline: 29 bytes */ }; > > >However that may not be true across all compilers etc. Also all the >other structures are __packed. Makes sense. > >Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> > [Suman] I agree, "having performance impact" is a wrong statement. I will update the same in v2.
From: Jacob Keller > Sent: 18 December 2023 20:44 > > On 12/18/2023 12:27 AM, Suman Ghosh wrote: > > Couple of structures was not marked as __packed which may have some > > performance implication. This patch fixes the same and mark them as > > __packed. > > Not sure I follow why lack of __packed would have performance > implications? I get that __packed is important to ensure layout is > correct or to ensure the whole structure has the right size rather than > unexpected gaps. I'd guess maybe because the structures size would > include padding without __packed, leading to a lot of gaps when > combining several structures together... > > I did test on my system with pahole, and even without __packed, I don't > get any gaps in the npc_lt_def_cfg structure: > > > > struct npc_lt_def_cfg { > > struct npc_lt_def rx_ol2; /* 0 3 */ > > struct npc_lt_def rx_oip4; /* 3 3 */ > > struct npc_lt_def rx_iip4; /* 6 3 */ > > struct npc_lt_def rx_oip6; /* 9 3 */ > > struct npc_lt_def rx_iip6; /* 12 3 */ > > struct npc_lt_def rx_otcp; /* 15 3 */ > > struct npc_lt_def rx_itcp; /* 18 3 */ > > struct npc_lt_def rx_oudp; /* 21 3 */ > > struct npc_lt_def rx_iudp; /* 24 3 */ > > struct npc_lt_def rx_osctp; /* 27 3 */ > > struct npc_lt_def rx_isctp; /* 30 3 */ > > struct npc_lt_def_ipsec rx_ipsec[2]; /* 33 10 */ > > struct npc_lt_def pck_ol2; /* 43 3 */ > > struct npc_lt_def pck_oip4; /* 46 3 */ > > struct npc_lt_def pck_oip6; /* 49 3 */ > > struct npc_lt_def pck_iip4; /* 52 3 */ > > struct npc_lt_def_apad rx_apad0; /* 55 4 */ > > struct npc_lt_def_apad rx_apad1; /* 59 4 */ > > struct npc_lt_def_color ovlan; /* 63 5 */ > > /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */ > > struct npc_lt_def_color ivlan; /* 68 5 */ > > struct npc_lt_def_color rx_gen0_color; /* 73 5 */ > > struct npc_lt_def_color rx_gen1_color; /* 78 5 */ > > struct npc_lt_def_et rx_et[2]; /* 83 10 */ > > > > /* size: 93, cachelines: 2, members: 23 */ > > /* last cacheline: 29 bytes */ > > }; > > > However that may not be true across all compilers etc. Also all the > other structures are __packed. Makes sense. Or not - maybe all the __packed should be removed instead! Unless these structures (or any others) appear in 'messages' which get transferred between systems they really shouldn't be __packed. And a 93 byte 'message' with all those fields seems rather odd. The above breakdown seems to imply everything is 'unsigned char' so the __packed makes no difference. Using __packed requires the compiler generate byte loads/store with shifts (etc) on many architectures and should really be avoided unless it is absolutely needed for binary compatibility. Even then if the problem is a 64bit field that only needs to be 32bit aligned (as is common for some compat32 code) then the 64bit fields should be marked as being 32bit aligned. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On 12/19/2023 7:26 AM, David Laight wrote: > From: Jacob Keller >> Sent: 18 December 2023 20:44 >> >> On 12/18/2023 12:27 AM, Suman Ghosh wrote: >>> Couple of structures was not marked as __packed which may have some >>> performance implication. This patch fixes the same and mark them as >>> __packed. >> >> Not sure I follow why lack of __packed would have performance >> implications? I get that __packed is important to ensure layout is >> correct or to ensure the whole structure has the right size rather than >> unexpected gaps. I'd guess maybe because the structures size would >> include padding without __packed, leading to a lot of gaps when >> combining several structures together... >> >> I did test on my system with pahole, and even without __packed, I don't >> get any gaps in the npc_lt_def_cfg structure: >> >> >>> struct npc_lt_def_cfg { >>> struct npc_lt_def rx_ol2; /* 0 3 */ >>> struct npc_lt_def rx_oip4; /* 3 3 */ >>> struct npc_lt_def rx_iip4; /* 6 3 */ >>> struct npc_lt_def rx_oip6; /* 9 3 */ >>> struct npc_lt_def rx_iip6; /* 12 3 */ >>> struct npc_lt_def rx_otcp; /* 15 3 */ >>> struct npc_lt_def rx_itcp; /* 18 3 */ >>> struct npc_lt_def rx_oudp; /* 21 3 */ >>> struct npc_lt_def rx_iudp; /* 24 3 */ >>> struct npc_lt_def rx_osctp; /* 27 3 */ >>> struct npc_lt_def rx_isctp; /* 30 3 */ >>> struct npc_lt_def_ipsec rx_ipsec[2]; /* 33 10 */ >>> struct npc_lt_def pck_ol2; /* 43 3 */ >>> struct npc_lt_def pck_oip4; /* 46 3 */ >>> struct npc_lt_def pck_oip6; /* 49 3 */ >>> struct npc_lt_def pck_iip4; /* 52 3 */ >>> struct npc_lt_def_apad rx_apad0; /* 55 4 */ >>> struct npc_lt_def_apad rx_apad1; /* 59 4 */ >>> struct npc_lt_def_color ovlan; /* 63 5 */ >>> /* --- cacheline 1 boundary (64 bytes) was 4 bytes ago --- */ >>> struct npc_lt_def_color ivlan; /* 68 5 */ >>> struct npc_lt_def_color rx_gen0_color; /* 73 5 */ >>> struct npc_lt_def_color rx_gen1_color; /* 78 5 */ >>> struct npc_lt_def_et rx_et[2]; /* 83 10 */ >>> >>> /* size: 93, cachelines: 2, members: 23 */ >>> /* last cacheline: 29 bytes */ >>> }; >> >> >> However that may not be true across all compilers etc. Also all the >> other structures are __packed. Makes sense. > > Or not - maybe all the __packed should be removed instead! > > Unless these structures (or any others) appear in 'messages' which > get transferred between systems they really shouldn't be __packed. > And a 93 byte 'message' with all those fields seems rather odd. > > The above breakdown seems to imply everything is 'unsigned char' > so the __packed makes no difference. > > Using __packed requires the compiler generate byte loads/store > with shifts (etc) on many architectures and should really be avoided > unless it is absolutely needed for binary compatibility. > > Even then if the problem is a 64bit field that only needs to be > 32bit aligned (as is common for some compat32 code) then the 64bit > fields should be marked as being 32bit aligned. > > David > Right. Typically packed is only required when dealing with something where the exact binary layout matters (i.e. copying to/from hardware or across systems in such a way that the layout might change with different compilers/arch). If that isn't how this structure is used, then ya, removing __packed seems reasonable. And at least for one system I see no difference in the actual generated layout, making __packed redundant. However, its not clear to me at a glance how this structure is used and whether it really is copied between places where binary compatibility is a requirement. > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
>>> >>> However that may not be true across all compilers etc. Also all the >>> other structures are __packed. Makes sense. >> >> Or not - maybe all the __packed should be removed instead! >> >> Unless these structures (or any others) appear in 'messages' which get >> transferred between systems they really shouldn't be __packed. >> And a 93 byte 'message' with all those fields seems rather odd. >> >> The above breakdown seems to imply everything is 'unsigned char' >> so the __packed makes no difference. >> >> Using __packed requires the compiler generate byte loads/store with >> shifts (etc) on many architectures and should really be avoided unless >> it is absolutely needed for binary compatibility. >> >> Even then if the problem is a 64bit field that only needs to be 32bit >> aligned (as is common for some compat32 code) then the 64bit fields >> should be marked as being 32bit aligned. >> >> David >> >Right. Typically packed is only required when dealing with something >where the exact binary layout matters (i.e. copying to/from hardware or >across systems in such a way that the layout might change with different >compilers/arch). > >If that isn't how this structure is used, then ya, removing __packed >seems reasonable. And at least for one system I see no difference in the >actual generated layout, making __packed redundant. > >However, its not clear to me at a glance how this structure is used and >whether it really is copied between places where binary compatibility is >a requirement. > >> - >> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, >> MK1 1PT, UK Registration No: 1397386 (Wales) [Suman] Yes, these structures are copied from firmware. It is needed to inform kernel about some parsing information required by hardware. That is the reason structures are packed and these two were missed.
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/npc.h b/drivers/net/ethernet/marvell/octeontx2/af/npc.h index ab3e39eef2eb..8c0732c9a7ee 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/npc.h +++ b/drivers/net/ethernet/marvell/octeontx2/af/npc.h @@ -528,7 +528,7 @@ struct npc_lt_def { u8 ltype_mask; u8 ltype_match; u8 lid; -}; +} __packed; struct npc_lt_def_ipsec { u8 ltype_mask; @@ -536,7 +536,7 @@ struct npc_lt_def_ipsec { u8 lid; u8 spi_offset; u8 spi_nz; -}; +} __packed; struct npc_lt_def_apad { u8 ltype_mask;